On Wed, Jun 13, 2012 at 12:48 PM, Myles C. Maxfield <span dir="ltr"><<a href="mailto:myles.maxfield@gmail.com" target="_blank">myles.maxfield@gmail.com</a>></span> wrote:<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<div><div>I think the fact that ICU thinks that a null pointer is an invalid string isn't necessarily an incorrect one. It's just a quirk of an interface to a library. I don't think that a good solution is to change the interface of ICU and try to upstream a patch to ICU</div>


</div></blockquote><div><br></div><div>Certainly not since not all browsers ship with their own ICU embedded inside.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<div><div>I see two different solutions:</div><div>1) Document the fact that, in order to use webkit, you need an implementation of malloc(0) that doesn't return null pointers. This way, the burden of solving this problem is pushed downstream.</div>


</div></blockquote><div><br></div><div>I don't think we should impose arbitrary requirements like this.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<div>

<div>2) Find some place along the every pipeline from malloc(0) to ICU, and arbitrarily modify the pointer to a non-zero value. This is probably the best solution, but a real fix of this nature requires finding all the places where pointers can be passed into ICU.</div>


</div></blockquote><div><br></div><div>We already have an abstraction layer for all ICU functions in WTF and WebCore/platform because not all ports use ICU (e.g. Qt port). It should be fairly easy to add an extra code there to check whether a null pointer is passed in or not and bail out as appropriate.</div>


<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>If I solve (via option 2) just my one particular occurrence of this problem, I see three different places to arbitrarily modify the pointer given to ICU:</div>


<div>1) change the m_copyData16 pointer in StringImpl to an arbitrary value, and check for that arbitrary value on destruction</div>

<div>2) change the characters() accessor to check if m_copyData16 is null, and return an arbitrary value if it is. This works because callers of characters() shouldn't ever delete the pointer nor dereference the pointer (since the string's length is 0)</div>




<div>3) check for null at the call site of the ICU function.</div><div><br></div><div>I (perhaps prematurely) uploaded a CL <a href="https://bugs.webkit.org/show_bug.cgi?id=88936" target="_blank">implementing</a> option 2. What do you think?</div>


</div></blockquote><div><br></div><div>Adding any code to characters() will likely impact performance because it tends to be used in hot code paths so I'd rather avoid that.</div><div><br></div><div>- Ryosuke</div><div>


<br></div></div>