[webkit-reviews] review denied: [Bug 5174] Add support for compiling on POSIX : [Attachment 4134] Updated patch

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Sun Oct 2 21:03:56 PDT 2005


Darin Adler <darin at apple.com> has denied Maciej Stachowiak <mjs at apple.com>'s
request for review:
Bug 5174: Add support for compiling on POSIX
http://bugzilla.opendarwin.org/show_bug.cgi?id=5174

Attachment 4134: Updated patch
http://bugzilla.opendarwin.org/attachment.cgi?id=4134&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Great direction to go, being portable to POSIX!

The switches to using __APPLE__ for some things that are currently WIN32 seem
great, although I'm not certain __APPLE__ is the best way to check for the Mac
OS X platform. But we can easily switch to something else with a global
replace, so it seems fine to use it for now.

On the other hand, I'm not at all happy with all the changes to use #if defined
X rather than #if X. Is there really a compiler in use where you get warnings
for using #if X when X is not defined? If so, then I suppose we can switch
uniformly to use #if defined X, but we've intentionally not used that style up
until now, and I don't want to change unless there's a good reason. Is this
some feature in a new version of gcc?

I'm also not clear on why the seemingly not-entirely-portable code is the
"else" case. Is this final case truly a "POSIX" case, or is it simply Linux
being treated as the default? I don't understand why pthread_getattr_np is more
portable than pthread_get_stack_addr_np. I sense Linux bias here, and similarly
in the config.h header. Maybe we should seek an ifdef about the presence of
some non-portable pthreads extensions, or invent our own and put it in
config.h.

I see some left-in fprintf and fflush calls. We don't want to land that, I
don't think.

Must the tm_year/tm_mon call go inside an __APPLE__ ifdef? I can see how it's
not needed on all platforms, but is it harmful on any?



More information about the webkit-reviews mailing list