Re: Comments on thread library

From: williamkempf_at_[hidden]
Date: 2001-09-09 15:51:07

--- In boost_at_y..., Jens Maurer <Jens.Maurer_at_g...> wrote:
> 
> Before the time runs out, I've looked briefly at threadcorrect1.zip.
> 
> The library should be accepted into boost.
> 
>  - Most of tss.cpp is wrapped into BOOST_HAS_WINTHREADS,
> but the #include "threadmon.hpp" is not, which makes the
> compilation fail (or is tss.cpp not supposed to be compiled
> on POSIX at all?)

No, this was a bug.  I'll fix it.

>  - xtime.hpp uses "boost/stdint.h", which is deprecated
> and fails with como on my Linux box.  Use "boost/cstdint.hpp"
> instead (need to fix uintXX_t usage in the code as well).

I'll fix this.
 
>  - Please add a newline at the end of once.hpp

Thanks.
 
>  - Something needs to be declared "extern C", or a wrapper added:
> "once.cpp", line 45: error: argument of type "void (*)()" is 
incompatible with
>           parameter of type "void (*)() C"
>         pthread_once(&flag, func);
>                             ^

Hmmm... needs more evaluation than I have right now, but I'll fix it 
today or tomorrow.
 
> "thread.cpp", line 103: error: argument of type "void *(*)(void *)" 
is
>           incompatible with parameter of type "void *(*)(void *) C"
>       int res = pthread_create(&m_thread, 0, &thread_proxy, &param);
>                                              ^

This'll be the same sort of thing.  I'll fix it.
 
> "tss.cpp", line 134: error: argument of type "void (*)(void *)" is
>           incompatible with parameter of type "void (*)(void *) C"
>       int res = pthread_key_create(&m_key, cleanup);
>                                            ^

And again.  You've got a stricter compiler than those that have been 
used thus far.
 
>  - If you need to add "extern C" to some function definitions, do 
make
> sure to give those implementation-detail functions "static" linkage.
> An unnamed namespace won't cut it, because the namespace is not
> coded in the function name for "extern C" functions.

Thanks for this info.
 
>  - I'm unsure why I get these:
> "tss.cpp", line 134: error: argument of type "void (*)(void *)" is
>           incompatible with parameter of type "void (*)(void *) C"
>       int res = pthread_key_create(&m_key, cleanup);
>                                            ^
> 
> "tss.cpp", line 135: error: identifier "__assert_fail" is undefined
>       assert(res == 0);
>       ^

This one is beyond me.  What compiler/platform so that others may 
figure it out?
 
> "tss.cpp", line 144: error: identifier "__assert_fail" is undefined
>       assert(res == 0);
>       ^
> 
>  - Please fix the indentation in timeconv.inl, it looks ugly
> with 8 char tabstops.  Oh yes, boost tends to avoid tabs at all.

I did as well, but something happened along the way where my editor 
was changed back to using tabs instead of spaces so I missed a few.  
I'll correct.
 
>  - I notice that thread.hpp includes pthread.h, but only
> thread.cpp includes "windows.h".  This is asymmetric, but probably
> required, and not too grave.

Not required, but highly desired.  Including pthread.h doesn't cause 
much issue to client code (and is needed unless we go to a pimpl 
design, which people complained about early in the development).  On 
the other hand, including Windows.h causes a lot of grief with 
several macros being declared that can cause problems in portable 
code, and I was easily able to avoid it.

Bill Kempf