This is the mail archive of the cygwin-developers 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]
Other format: [Raw text]

Re: sem_init() fails (when used in a certain way)


On Apr  4 13:23, Jon TURNEY wrote:
> On 30/03/2011 06:35, Christopher Faylor wrote:
> > On Tue, Mar 29, 2011 at 11:38:32PM +0200, Corinna Vinschen wrote:
> >> You can't just replace all is_good_object tests with myfault handlers,
> >> afaics.  The only case where the is_good_object test doesn't make sense
> >> for the reason outlined in Jon's mail are the init methods of the
> >> various object types.  In all other methods the is_good_object test is
> >> still necessary to check the object pointer and to generate the EINVAL
> >> error code reliably.  So the myfault handler could (and probably
> >> should) be added to these methods while keeping the is_good_object
> >> test.
> > 
> > The reason I mentioned putting them in the functions rather than an init
> > function is to catch any subsequent problems with dereferencing invalid
> > pointers.  If you put a handler in the init function then it is only
> > valid for the life of the init function.  I wasn't suggesting replacing
> > all of the is_good_object tests, though, just the one that Jon
> > identified.
> 
> I don't think I understand what you are suggesting here.
> 
> That seems to be about the orthogonal issue of how sem_init() should deal with
> being handed an invalid pointer.  is_good_object() uses efault, but only in so
> far as if it's an invalid pointer, it can't be a good object, so sem_init()
> trundles on and tries to store the address of the newly allocated semaphore
> object through that invalid pointer :-)
> 
> Attached is a minimal patch to fix the issue I've identified with sem_init.  I
> haven't looked at the other classes in that file.
> 
> I am suspicious of arguments that this should be changed some other way 'for
> performance' which aren't backed up by profiling data.

> 2011-04-02  Jon TURNEY  <...>
> 
> 	* thread.cc (semaphore::init): We cannot reliably infer anything from
> 	the existing contents of sem, so merely warn rather than return EBUSY
> 	if it looks like we are reinitialising a semaphore.

I think you should check this in.  It's definitely the right thing to do
for a start, and it doesn't hinder us to reevaluate the issue at some
later point.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]