PTHREAD_COND_INITIALIZER
Robert Collins
robert.collins@itdomain.com.au
Mon Sep 24 15:01:00 GMT 2001
Hi Egor,
Thanks for that.
It's not quite right yet though....
In pthread.h,
* PTHREAD_COND_INITIALIZER must be a never valid address, that is
unlikely to occur randomly - 0 won't work. (See
PTHREAD_MUTEX_INITIALIZER).
* I'm still not sure of the appropriateness of the clean_up routine
typedef change, it should go in as a separate patch regardless.
In thread.cc, you have introduced many deference potential invalid
memory issues.
No dereferencing of * parameters should occur until a
verifyable_object_isvalid () call is made on them. As you can see that
function checks for address space allocation and for the INITIALIZER
macro's, and for the correct contect according to a magic cookie.
ie this is bad:
+ if (*cond != PTHREAD_COND_INITIALIZER &&
+ verifyable_object_isvalid (*cond, PTHREAD_COND_MAGIC))
return EBUSY;
whereas adding PTHREAD_COND_INITIALIZER to verifyable_object_isvalid
won't introduce this issue.
pthread_cond_construct is a (to me) unneeded function, thats what the
object constructor is for. Finally reusing the verifyyobject call will
result in less os overhead (in calls to IsBadWritePtr).
Can I suggest you look at the implementation of
PTHREAD_MUTEX_INITIALIZER, PTHREAD_COND_INITIALIZER should be nearly
identical.
(ie the new code in each function is only two lines:
int
__pthread_mutex_lock (pthread_mutex_t *mutex)
{
if (!verifyable_object_isvalid (*themutex, PTHREAD_MUTEX_MAGIC))
return EINVAL;
== new code ==
if (*mutex == PTHREAD_MUTEX_INITIALIZER)
__pthread_mutex_init (mutex, NULL);
== /new code ==
(*themutex)->Lock ();
return 0;
}
And finally, can you create a simple (able to be included in my GPL'd
pthread test suite) testcase to test the COND_INITIALIZER? If you have
the time to create boundary tests as well, that's even better, but a
core test is needed IMO. I'll run the whole suite for regression
checking (unless you want to :} ).
Rob
----- Original Message -----
From: "egor duda" <deo@logos-m.ru>
To: <cygwin-patches@cygwin.com>
Sent: Tuesday, September 25, 2001 1:39 AM
Subject: PTHREAD_COND_INITIALIZER
> Hi!
>
> attached is a patch which adds support for PTHREAD_COND_INITIALIZER.
> please don't be fooled by
>
> -__pthread_cond_destroy (pthread_cond_t *cond)
> +__pthread_cond_construct (pthread_cond_t *cond)
>
> lines, it's 'diff', not me :)
>
> egor. mailto:deo@logos-m.ru icq 5165414 fidonet
2:5020/496.19
More information about the Cygwin-patches
mailing list