Summary: | Add QNX as supported platform | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Harald Fernengel <harry> | ||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Enhancement | CC: | hausmann | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | Other | ||||||||
Attachments: |
|
Description
Harald Fernengel
2009-07-31 09:22:28 PDT
Comment on attachment 33880 [details] Patch making WebKitCore and JavaScriptCore compile on QNX The patch looks good to me, but I'm wondering about the hunk below: > @@ -544,8 +567,9 @@ static inline void* currentThreadStackBa > #endif > int rc = pthread_attr_getstack(&sattr, &stackBase, &stackSize); > (void)rc; // FIXME: Deal with error code somehow? Seems fatal. > - ASSERT(stackBase); > pthread_attr_destroy(&sattr); > +#endif > + ASSERT(stackBase); > stackThread = thread; > } > return static_cast<char*>(stackBase) + stackSize; Why did you move the ASSERT? :) Comment on attachment 33880 [details]
Patch making WebKitCore and JavaScriptCore compile on QNX
Can't we use a ThreadingQNX abstraction here to get the stack base instead?
Comment on attachment 33880 [details]
Patch making WebKitCore and JavaScriptCore compile on QNX
We have LOG macros for this:
+#ifndef NDEBUG
+ perror("Unable to open /proc/self:");
+#endif
No c-style casts in c++:
+ stackBase = (void*)threadInfo.stkbase;
r-. Please explain why this shouldn't use (or expand and then use) the Threading abstraction model in Threading*
Created attachment 34446 [details]
Updated version of the QNX patch
Thanks for your comments.
I've removed the C-style cast and used LOG_ERROR instead of perror().
Also, the QNX specific part of getting the current thread's stack base is now factored out into currentThreadStackBaseQNX(), making it more readable and removing some #define magic.
QNX offers standard pthreads, so a special Threading* abstraction is not necessary. Collector.cpp already contains #defines for all platforms that use standard pthreads but don't offer pthread_getattr_np() to retrieve the stack base.
Comment on attachment 34446 [details] Updated version of the QNX patch r=me > +static inline void *currentThreadStackBaseQNX() Small coding style buglet here, but I'll fix that when landing. |