[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