From: Erik Faye-Lund <kusmabite@googlemail.com>
Date: Wed, 30 Dec 2009 13:47:11 GMT
Subject: Re: [Updated PATCH 1/2] Report exec errors from run-command
Message-ID: <40aa078e0912300547r407efb74xb821345a54a87277@mail.gmail.com>
In-Reply-To: <1262170338-11574-2-git-send-email-ilari.liusvaara@elisanet.fi>

On Wed, Dec 30, 2009 at 11:52 AM, Ilari Liusvaara
<ilari.liusvaara@elisanet.fi> wrote:
> +static inline void force_close(int fd)
> +{
> +       while (close(fd) < 0 && errno != EBADF)
> +               ; /* No-op */
> +}
> +

According to http://linux.die.net/man/2/close, close can set errno to
EBADF, EINTR, or EIO. Currently, you're retrying on EINTR and EIO.
When we get EIO, are you sure it makes sense to retry? I'd imagine
that error would most likely just repeat itself, leading to an
infinite loop. How about "while (close(fd) < 0 && errno == EINTR)"
instead? I've seen other functions (like xread in wrapper.c) only
retry on those errors that it expects. In xreads case, it's not
retrying on EIO.

Perhaps it's OK still, since force_close() is only used on pipes. I
don't know if closing a pipe can generate EIO or not.

-- 
Erik "kusma" Faye-Lund