[systemd-devel] [PATCH 12/24] sd-dhcp6-client: Add DHCPv6 Solicit message creation and sending

[Zbigniew Jędrzejewski-Szmek]

On Fri, Jun 20, 2014 at 07:45:39PM +0200, Lennart Poettering wrote:
> On Thu, 19.06.14 14:52, Patrik Flykt (Patrik.Flykt at linux.intel.com) wrote:
> 
> > On Wed, 2014-06-18 at 16:25 +0200, Zbigniew Jędrzejewski-Szmek wrote:
> > > On Wed, Jun 18, 2014 at 07:05:35AM -0700, Filipe Brandenburger wrote:
> > > > On Wed, Jun 18, 2014 at 6:58 AM, Zbigniew Jędrzejewski-Szmek
> > > > <zbyszek at in.waw.pl> wrote:
> > > > >> +        if (client->fd > 0)
> > > > >> +                safe_close(client->fd);
> > > > >> +        client->fd = -1;
> > > > > client->fd = safe_close(client->fd);
> > > > >
> > > > > That's what safe_close is for :)
> > > > 
> > > > And shouldn't the check be for client->fd >= 0? Zero is a valid file descriptor.
> > > Yeah... but note that safe_close already has the fd >= 0 check, so the
> > > replacement line replaces the if too.
> > 
> > If I omit the client->fd > 0 check I get the following in the test case:
> > 
> > Assertion 'close_nointr(fd) != -EBADF' failed at src/shared/util.c:205,
> > function safe_close(). Aborting.
> 
> safe_close() is a call that will ignore all kinds of errors except one:
> EBADF. THe rationale for that is that the kernel has these weird
> semantics that close() might fail with errors like EIO or whatever else
> if something could't be written to disk or so, but the fd will still be
> closed. Hence, you can invoke safe_close() in all those cases where you
> just want to get rid of an fd, and don't care about anything else. Now,
> it will only trip up on one specific problem which always indicates a
> programming error: when close() returns EBADF, since that is the error
> that is returned when you invoke close() on an fd that doesn't exist.
> 
> Putting this all together: safe_close() is basically your one stop
> solution to getting rid of fds, and even updating your variable you
> store it in:
> 
> fd = safe_close(fd);
> 
> safe_close() always returns -1, always gets rid of the fd, will be a NOP
> if the fd is < 0. Will never fail. However, it if you invoke it the only
> way that is a real programming error which is with an already-closed fd
> or a never-opened fd, then it will assert() and die.
> 
> Hope this makes sense.
> 
> Or long words short: if the code tripped up like yours above, this is no
> indication that safe_close() wasn't right. Instead it's an indication
> that you are passing it a rubbish fd.
Yeah, fixed in c806ffb.

Zbyszek