Re: [PATCH 3/5] libports: fix error handling in _ports_create_port_internal

[Justus Winter]

Quoting Thomas Schwinge (2013-10-25 15:27:10)
> > --- a/libports/create-internal.c
> > +++ b/libports/create-internal.c
> > @@ -109,10 +109,11 @@ _ports_create_port_internal (struct port_class *class,
> >    err = EINTR;
> >   lose:
> >    pthread_mutex_unlock (&_ports_lock);
> > +  error_t e;
> >   lose_unlocked:
> > -  err = mach_port_mod_refs (mach_task_self (), port,
> > -                         MACH_PORT_RIGHT_RECEIVE, -1);
> > -  assert_perror (err);
> > +  e = mach_port_mod_refs (mach_task_self (), port,
> > +                       MACH_PORT_RIGHT_RECEIVE, -1);
> > +  assert_perror (e);
> >    free (pi);
> >  
> >    return err;
> 
> OK, but I suggest to move the definition of e after the lose_unlocked
> label, 

I know it looks strange, but what you're suggesting is not
possible. Labels can only be placed in front of a statement and a
declaration is not a statement. Well, that's what gcc told me...

> or even move all that (mach_port_mod_refs call and assert_perror)
> into a new scope to make its locality clear.  May also want to replace
> the earlier »if (!  pi)« handling with »goto lose_unlocked;« (»free
> (NULL)« being a no-op).

Ok, I will have a look at cleaning up this function. However, this
seems like a critical bug in a critical component of the Hurd. I
wanted to post this now as Samuel is planning to upload a new Hurd
package soonish. I have tested this patch series as it is. It is
running on my development box and that has almost completed a hurd
rebuild by now.

I will be afk for the weekend, if anyone wants to improve the patch in
the meantime, I won't mind ;)

Cheers,
Justus