Thanks for the quick responses and the bug link about fastMalloc(0). In comment 11 <https://bugs.webkit.org/show_bug.cgi?id=55097#c11>, Eric Seidel says that he was going to start a discussion on webkit-dev, but I haven't been able to find such a conversation (by looking both on gmane and on google). All I've been able to find is this thread<https://lists.webkit.org/pipermail/webkit-dev/2011-June/016967.html>, which is only a single post. Does anyone know where this conversation takes place?
That being said, Darin Adler is right; the original patch was intended to improve performance, not because something was breaking. It seems like most of the conversation regarding that patch is about any possible performance increase. In addition, Eric Seidel's patch was to guarantee that no one tries to call fastMalloc with a size of 0; I'm merely concerned about the value of the pointer returned when someone does call that. 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 - I think a better solution would be to work around this "requirement" of ICU inside WebKit. 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. 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. 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? --Myles On Wed, Jun 13, 2012 at 2:09 AM, Zoltan Horvath <zol...@webkit.org> wrote: > > Hi, > > The bug report about fastMalloc(0): > https://bugs.webkit.org/show_bug.cgi?id=55097 > > Brewmp had conditions for fastMalloc(0) earlier, but it was removed in: > http://trac.webkit.org/changeset/95555/trunk/Source/JavaScriptCore/wtf/FastMalloc.cpp > > Cheers, > <Zoltan> > > > On Wed, 13 Jun 2012 00:08:48 +0200, Adam Barth <aba...@webkit.org> wrote: > >> There was some discussion about how to handle malloc(0) a year or so >> ago. I don't remember if it was on webkit-dev, but you might want to >> check the archives. Eric Seidel might remember what conclusions (if >> any) we came to. >> >> Adam >> >> >> On Tue, Jun 12, 2012 at 3:03 PM, Myles C. Maxfield >> <myles.maxfi...@gmail.com> wrote: >>> >>> Hello, >>> I'm compiling WebKit with a malloc() implementation that returns NULL >>> for malloc(0). According to C99, this is valid: "If the size of the >>> space requested is zero, the behavior is implementation- deļ¬ned: >>> either a null pointer is returned, or the behavior is as if the size >>> were some nonzero value, except that the returned pointer shall not be >>> used to access an object." >>> >>> I noticed that this caused a problem in one particular place >>> (WTF::StringImpl::getData16SlowCase()) where the code allocates >>> (constant * length) bytes for an (empty) string, and provides an >>> accessor that exposes this pointer. This pointer was being passed to >>> ICU, which didn't perform the requested function because it looked >>> like one of the arguments was invalid, even though it was just empty. >>> >>> I have worked around this one particular occurrence in my local >>> version of WebKit fork, but I'm wondering how often this pattern >>> occurs. Is my fix worth upstreaming? Is it worth trying to find, >>> "fix," and upstream every occurrence of this pattern? Or is this >>> particular behavior of malloc() an unstated requirement of building >>> WebKit? If the latter is true, perhaps it's worth explicitly stating >>> this somewhere? What is the opinion of the community? >>> >>> Thanks, >>> Myles >>> _______________________________________________ >>> webkit-dev mailing list >>> webkit-dev@lists.webkit.org >>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev >> >> _______________________________________________ >> webkit-dev mailing list >> webkit-dev@lists.webkit.org >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > > _______________________________________________ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
