Boost logo

Boost :

From: williamkempf_at_[hidden]
Date: 2001-03-15 11:20:17


--- In boost_at_y..., Doug Gregor <gregod_at_r...> wrote:
> 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>

Wow! Hard to believe I missed this many of them! Thanks for finding
them.

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

These particular ones occurred because the code originally existed in
a Windows specific implementation file. At least I have an excuse
for two of them ;).
 
> 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>

And I just didn't pay as much attention to the examples as I did the
code...
 
> > > 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.

Thanks, you've confirmed my own experience. Definately a subtle race
condition in the implementation. I'll have to see if I can track
down what it is.
 
> > > 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 };

I'll do this.
 
> Here's the list of nitpicks the Comeau compiler found:
> - Can't use a ';' after '}' ending a namespace (happens everywhere)

My own misunderstanding of C++ syntax. I'll fix this.

> - Add an extra line feed at the end of each line (happens
everywhere)

This one I don't understand. Is it a problem with the \r\n
DOS/Windows line endings?

> - #include <ctime> to get std::clock_t. (condition.hpp)

Thanks.

> - Qualify clock_t as std::clock_t (condition.hpp, src/*.cpp)

Won't work with VC++, so I'll have to use the usual work arounds.
Thanks.

> - Nested "impl" classes have to be declared friends of their
enclosing
> classes (semaphore.hpp, mutex.hpp, fast_mutex.hpp,
recursive_mutex.hpp)

This one I don't buy. The "nested class" does not use the "enclosing
class" in any way while the "enclosing class" uses only public
methods of the "nested class". I see no need for friendship in
either direction.

> - use 0 instead of NULL because many compilers pick up nonportable
NULL
> definitions (fast_mutex.cpp, mutex.cpp, recursive_mutex.cpp,
semaphore.cpp)

Dang... this is one that I try not to do but bad habits let some slip
through. I'll search out and fix this one as well.

> - 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.

Thanks. This code will be replaced with proper Boost alternatives in
Phase 2, but I should make the "hack" legal code until then.

Again, thanks for the comments and pointers. I'll be working on this
stuff today and hopefully will have an update posted tonight or
tomorrow incorporating things people have mentioned here.

Bill Kempf


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk