[webkit-dev] malloc(0)

[Myles C. Maxfield]

There is still the question of if I can upstream this to ICU, which I'm
checking on.

In the meantime, Ryosuke Niwa has a good argument for option 3) above. I
have uploaded a new patch to
https://bugs.webkit.org/show_bug.cgi?id=88936that moves the NULL
pointer check to inside the ICU-specific collator class.

On Wed, Jun 13, 2012 at 1:14 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

> On Wed, Jun 13, 2012 at 12:48 PM, Myles C. Maxfield <
> myles.maxfield at gmail.com> wrote:
>
>> 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
>>
>
> Certainly not since not all browsers ship with their own ICU embedded
> inside.
>
> I see two different solutions:
>> 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.
>>
>
> I don't think we should impose arbitrary requirements like this.
>
>
>>  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.
>>
>
> 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.
>
> 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:
>> 1) change the m_copyData16 pointer in StringImpl to an arbitrary value,
>> and check for that arbitrary value on destruction
>> 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)
>> 3) check for null at the call site of the ICU function.
>>
>> I (perhaps prematurely) uploaded a CL implementing<https://bugs.webkit.org/show_bug.cgi?id=88936>option 2. What do you think?
>>
>
> 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.
>
> - Ryosuke
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120613/2aed56e9/attachment.html>