Bug 25325 - Make sure pthread_self() is declared before it gets called
Summary: Make sure pthread_self() is declared before it gets called
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-22 08:53 PDT by Laszlo Gombos
Modified: 2009-05-14 07:27 PDT (History)
1 user (show)

See Also:


Attachments
Proposed fix to include pthread.h in most Unix-like platforms (not just for OPENBSD) in Collector.cpp. (1012 bytes, patch)
2009-04-22 08:59 PDT, Laszlo Gombos
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2009-04-22 08:53:05 PDT
On some Unix-like platforms Collector.cpp fails to compile because pthread.h does not get included. The problem might be specific to those WebKit ports where neither WTF_USE_PTHREADS nor JSC_MULTIPLE_THREADS is defined (e.g. Qt port on Linux/FREEBSD/NETBSD).

The currentThreadStackBase() in Collector.cpp calls pthread_self() on most Unix-like platforms. OPENBSD includes pthread.h explicitly but none of the other Unix-like platforms do so. On the Qt Linux WebKit build, pthread.h gets implicitly included from algorithm, which makes the Qt Unix-like builds dependent on a particular STL implementation.
Comment 1 Laszlo Gombos 2009-04-22 08:59:43 PDT
Created attachment 29680 [details]
Proposed fix to include pthread.h in most Unix-like platforms (not just for OPENBSD) in Collector.cpp.
Comment 2 Holger Freyther 2009-05-11 20:37:37 PDT
Comment on attachment 29680 [details]
Proposed fix to include pthread.h in most Unix-like platforms (not just for OPENBSD) in Collector.cpp.


>  
>  #if PLATFORM(SOLARIS)
>  #include <thread.h>
> -#endif
> -
> -#if PLATFORM(OPENBSD)
> +#else
>  #include <pthread.h>
>  #endif
>  

I'm not convinced why including this for WINDOWS and DARWIN is a good idea.
Comment 3 Laszlo Gombos 2009-05-11 20:46:52 PDT
(In reply to comment #2)
> (From update of attachment 29680 [details] [review])
> >  
> >  #if PLATFORM(SOLARIS)
> >  #include <thread.h>
> > -#endif
> > -
> > -#if PLATFORM(OPENBSD)
> > +#else
> >  #include <pthread.h>
> >  #endif
> >  
> I'm not convinced why including this for WINDOWS and DARWIN is a good idea.

This entire code segment is guarded with 

#if PLATFORM(DARWIN)
#elif PLATFORM(WIN_OS)
#elif PLATFORM(UNIX)

so pthread.h won't be included for WIN_OS and DARWIN. 

I realize that the diff does not show enough context in this case. With this additional information, I will put up the patch for review again.



Comment 4 Darin Adler 2009-05-12 07:48:17 PDT
Comment on attachment 29680 [details]
Proposed fix to include pthread.h in most Unix-like platforms (not just for OPENBSD) in Collector.cpp.

Seems OK, r=me

But maybe HAVE_PTHREAD_H would be an even more reliable way of doing this.
Comment 5 Holger Freyther 2009-05-14 07:27:06 PDT
Landed in r43698.