Re: [boost] Re: Boost.Threads draft library submission

From: Doug Gregor (gregod_at_[hidden])
Date: 2001-03-15 10:59:54

On Thursday 15 March 2001 09:30, you wrote:
[snip]
> > It builds fine on x86 Linux with a few minor changes:
> > 	1) Use forward slashes instead of backslashes when including
>
> headers.
>
> > Windows will support #include <boost/header.hpp>, but not all Unix
>
> systems
>
> > (Linux included) support #include <boost\header.hpp>.
>
> Jeremy Siek already pointed this out to me and I attempted to change
> all of them.  Which specific ones did I miss?

The from threads/boost subdirectory:
atomic.hpp:21:#include <boost\config.hpp>
atomic.hpp:27:#   include <boost\fast_mutex.hpp>
condition.hpp:21:#include <boost\config.hpp>
fast_mutex.hpp:21:#include <boost\config.hpp>
fast_mutex.hpp:26:#include <boost\utility.hpp>
fast_mutex.hpp:27:#include <boost\xlock.hpp>
mutex.hpp:21:#include <boost\config.hpp>
mutex.hpp:26:#include <boost\utility.hpp>
mutex.hpp:27:#include <boost\xlock.hpp>
recursive_mutex.hpp:21:#include <boost\config.hpp>
recursive_mutex.hpp:26:#include <boost\utility.hpp>
recursive_mutex.hpp:27:#include <boost\xlock.hpp>
semaphore.hpp:21:#include <boost\config.hpp>
semaphore.hpp:26:#include <boost\utility.hpp>
thread.hpp:21:#include <boost\config.hpp>

From the threads/src directory:
condition.cpp:21:#   include <boost\semaphore.hpp>
condition.cpp:22:#   include <boost\atomic.hpp>

From the threads/example directory:
monitor.cpp:3:#include <boost\condition.hpp>
monitor.cpp:4:#include <boost\fast_mutex.hpp>
monitor.cpp:5:#include <boost\mutex.hpp>
monitor.cpp:6:#include <boost\recursive_mutex.hpp>

>
> > 	2) xlock.hpp has an extra "typename" in line 61 that GCC
>
> complains about.
>
> Another one that Jeremy pointed out and I must have missed.  I'll
> correct both of these yet today.
>
> > However, I have reproduced the deadlock problem with the example
>
> program.
>
> With what sort of frequency?  Do you have any way of telling where it
> deadlocks for me?  Obviously I've got a race condition here and will
> have to track it down and fix it.

Out of 100 runs, it deadlocked three times. I don't know of any easy way of 
determining where the deadlock occurs, but I'll take a closer look at it 
later.

> > A compiler at GCC's highest warning level (-ansi -pedantic -Wall)
>
> results in
>
> > some signed/unsigned comparison warnings in the pthread handling
>
> code.
>
> > Commonly the condition:
> >
> > if (milliseconds == -1)
> >
> > appears, where "milliseconds" is an unsigned int. Is this
>
> safe/intended?
>
> Intended.  I'm not 100% sure about safe.  If this is not portably
> safe I'll need to change this to use numeric_limits<>::max.

I'm not sure how to decipher what the C++ standard says in this regard 
(4.7/2), though it appears that it won't be safe if the architecture uses 
something other than two's complement integer arithmetic. My personal opinion 
is that it would be best to replace -1 with, e.g., 
enum { FOREVER = std::numeric_limits<unsigned int>::max };

Here's the list of nitpicks the Comeau compiler found:
- Can't use a ';' after '}' ending a namespace (happens everywhere)
- Add an extra line feed at the end of each line (happens everywhere)
- #include <ctime> to get std::clock_t. (condition.hpp)
- Qualify clock_t as std::clock_t (condition.hpp, src/*.cpp)
- Nested "impl" classes have to be declared friends of their enclosing 
classes (semaphore.hpp, mutex.hpp, fast_mutex.hpp, recursive_mutex.hpp)
- use 0 instead of NULL because many compilers pick up nonportable NULL 
definitions (fast_mutex.cpp, mutex.cpp, recursive_mutex.cpp, semaphore.cpp)
- In the monitor example, a "void*" parameter is casted to "void (*)()", 
which is illegal. The attached monitor.cpp uses a simple dispatch object to 
get around this limitation.

        Doug