On Fri, Jun 20, 2014 at 07:45:39PM +0200, Lennart Poettering wrote: > On Thu, 19.06.14 14:52, Patrik Flykt (patrik.fl...@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 > > > > <zbys...@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 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
