[webkit-reviews] review denied: [Bug 49479] Building QtWebKit fails on uClibc (0.9.31) : [Attachment 73782] Fixed code styling complains
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Nov 17 03:43:48 PST 2010
Andreas Kling <kling at webkit.org> has denied Daniel Nyström
<daniel.nystrom at timeterminal.eu>'s request for review:
Bug 49479: Building QtWebKit fails on uClibc (0.9.31)
https://bugs.webkit.org/show_bug.cgi?id=49479
Attachment 73782: Fixed code styling complains
https://bugs.webkit.org/attachment.cgi?id=73782&action=review
------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=73782&action=review
> JavaScriptCore/ChangeLog:5
> + Merged changes from QtScript which fixes missing pthread_getattr_np
not available on uClibc causing build error.
This is a bit muddy. Suggestion:
==
Fix currentThreadStackBase() compilation against uClibc
uClibc doesn't have pthread_getattr_np or pthread_attr_getstack.
Patch originally from QtScript.
==
> JavaScriptCore/runtime/Collector.cpp:77
> +// versions of uClibc 0.9.31 and below do not have
> +// pthread_getattr_np or pthread_attr_getstack.
This seems a bit strange to me, since 0.9.31 is the latest version (AFAICT from
uclibc.org)
Shouldn't we be doing this for *any* uClibc version, not just <= 0.9.31?
> JavaScriptCore/runtime/Collector.cpp:544
> + size_t lineLen = 0;
We generally don't abbreviate in WebKit, "lineLength" sounds better here.
> JavaScriptCore/runtime/Collector.cpp:551
> + if (sscanf (line, "%lx-%lx", &from, &to) != 2)
Style, remove space after sscanf.
> JavaScriptCore/runtime/Collector.cpp:557
> + return (void *)from;
Style, remove space after void.
> JavaScriptCore/runtime/Collector.cpp:559
> + return (void *)to;
Ditto.
More information about the webkit-reviews
mailing list