This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
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
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |