[webkit-reviews] review denied: [Bug 63717] Broken build on QNX : [Attachment 99300] patch

[bugzilla-daemon at webkit.org]

Patrick R. Gansterer <paroga at paroga.com> has denied Ademar Reis
<ademar.reis at openbossa.org>'s request for review:
Bug 63717: Broken build on QNX
https://bugs.webkit.org/show_bug.cgi?id=63717

Attachment 99300: patch
https://bugs.webkit.org/attachment.cgi?id=99300&action=review

------- Additional Comments from Patrick R. Gansterer <paroga at paroga.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=99300&action=review

> Source/JavaScriptCore/heap/MachineStackMarker.cpp:83
> +#if !OS(QNX)
> +/* QNX doesn't support SA_RESTART yet. */

Are you sure you want ENABLE(JSC_MULTIPLE_THREADS)?
Why not disable JSC_MULTIPLE_THREADS if you can't support it correctly?

> Source/JavaScriptCore/wtf/Platform.h:663
> +#if OS(QNX)
> +#define ENABLE_JIT 0
> +#endif

Why do you put it in it's own #if?
IMHO
http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/wtf/Platform.h?rev=9
0167#L1027 is the correct place for this.

> Source/JavaScriptCore/wtf/StackBounds.cpp:49
> +#include <pthread.h>
>  #include <fcntl.h>
>  #include <sys/procfs.h>
>  #include <stdio.h>
>  #include <errno.h>
> +#include <string.h>

I agree with the false positive, but please sort the headers.