From: Jim Meyering <jim@meyering.net>
Date: Thu, 26 May 2011 14:37:10 GMT
Subject: Re: [PATCH] remove unnecessary test and dead diagnostic
Message-ID: <87hb8h5y09.fsf@rho.meyering.net>
In-Reply-To: <20110526141130.GB18520@sigill.intra.peff.net>

Jeff King wrote:
...
>   $ sed -n '/read_in_full/,/^}/p' wrapper.c
>   ssize_t read_in_full(int fd, void *buf, size_t count)
>   {
>           char *p = buf;
>           ssize_t total = 0;
>
>           while (count > 0) {
>                   ssize_t loaded = xread(fd, p, count);
>                   if (loaded <= 0)
>                           return total ? total : loaded;
>                   count -= loaded;
>                   p += loaded;
>                   total += loaded;
>           }
>
>           return total;
>   }
>
> It looks like if we get -1 on the _first_ read, we will then return -1.
> Subsequent errors are then ignored, and we return the (possibly
> truncated) result.
>
> Which, to be honest, seems kind of insane to me. I'd think:
>
>   while (count > 0) {
>           ssize_t loaded = xread(fd, p, count);
>           if (loaded < 0)
>                   return loaded;
>           if (loaded == 0)
>                   return total;
>           ...
>   }
>
> would be much more sensible semantics.

That looks better to me, too.