egor duda
Mon Sep 24 23:55:00 GMT 2001


Tuesday, 25 September, 2001 Robert Collins wrote:

RC> Hi Egor,
RC> Thanks for that.

RC> It's not quite right yet though....
RC> In pthread.h,
RC> * PTHREAD_COND_INITIALIZER must be a never valid address, that is
RC> unlikely to occur randomly - 0 won't work. (See

why? SUSv2 doesn't tell anything about PTHREAD_COND_INITIALIZER's
validity as pointer. Are there any particular reasons for not making
it NULL?

RC> * I'm still not sure of the appropriateness of the clean_up routine
RC> typedef change,

see pthread_cleanup_push() prototype in SUSv2. it reads:

void pthread_cleanup_push(void (*routine)(void*), void *arg);

current macro won't compile if called from application this way.

RC> it should go in as a separate patch regardless.

RC> In, you have introduced many deference potential invalid
RC> memory issues.  No dereferencing of * parameters should occur until a
RC> verifyable_object_isvalid () call is made on them. As you can see that
RC> function checks for address space allocation and for the INITIALIZER
RC> macro's, and for the correct contect according to a magic cookie.

RC> ie this is bad:
RC> + verifyable_object_isvalid (*cond, PTHREAD_COND_MAGIC))
RC> return EBUSY;

only one, actually. *cond is called unchecked only in
__pthread_cond_init(), and i'll fix that. Other functions call
__pthread_cond_construct, which, in turn, check if cond is a valid
pointer. Maybe __pthread_cond_construct() is misleading name as in
do both verification and construction of condvar object, and i open
for suggestions about better name.

RC> whereas adding PTHREAD_COND_INITIALIZER to verifyable_object_isvalid
RC> won't introduce this issue.

RC> pthread_cond_construct is a (to me) unneeded function, thats what the
RC> object constructor is for. Finally reusing the verifyyobject call will
RC> result in less os overhead (in calls to IsBadWritePtr).

btw, i don't like making PTHREAD_MUTEX_INITIALIZER to be (void*) 20.
it's not guaranteed by anybody that this memory area won't be
accessible. by using (void*) 20 we add yet another dependency on
undocumented (though always working, for now) system feature. Cygwin
has a plenty of them already, no need to add one more where it's not

RC> Can I suggest you look at the implementation of
RC> identical.
RC> (ie the new code in each function is only two lines:

that's reasonable. i think that "PTHREAD_*_INITIALIZER"s are better
made to be NULL.

RC> And finally, can you create a simple (able to be included in my GPL'd
RC> pthread test suite) testcase to test the COND_INITIALIZER?

do you have one? would you mind including it in cygwin testsuite in

RC> If you have  the time to create boundary tests as well, that's even better, but a
RC> core test is needed IMO. I'll run the whole suite for regression
RC> checking (unless you want to :} ).

i'm porting testsuite from pthreads-win32 to cygwin and want to add it
to current cygwin testsuite.

Egor.   ICQ 5165414 FidoNet 2:5020/496.19

More information about the Cygwin-patches mailing list