[webkit-reviews] review denied: [Bug 20746] Port WebKit to Qt on Windows CE : [Attachment 23469] Change DateMath.cpp to not use errno with strtol (updated)

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Joerg Bornemann
<joerg.bornemann at trolltech.com>'s request for review:
Bug 20746: Port WebKit to Qt on Windows CE
https://bugs.webkit.org/show_bug.cgi?id=20746

Attachment 23469: Change DateMath.cpp to not use errno with strtol (updated)
https://bugs.webkit.org/attachment.cgi?id=23469&action=edit

------- Additional Comments from Eric Seidel <eric at webkit.org>
This is nice.  2 comments though.

This will crash if endPtr is null:
+    return string != *endPtr && retVal != LONG_MIN && retVal != LONG_MAX;

Maybe strtol would also crash in that situation, not sure?

Also, since this is our own function:
+static bool parseLong(long& retVal, const char* string, char** endPtr, int
base)

I think it's clearer (less typing at least), to use:
+static bool parseLong(long& retVal, const char* string, char*& endPtr, int
base)

Then one doesn't need to add & to every callsite.

We also could make base =10 a default paramteter to further clean up the
callsites.

I think this patch is really OK as is, but given that we're in this code, I
feel like we should clean it up a bit more.  I guess I could do that and post a
response patch for you.  I'll do that.