From: Andy Whitcroft <apw@shadowen.org>
Date: Fri, 05 Jan 2007 12:20:00 GMT
Subject: Re: [PATCH] use xread where we are not checking for EAGAIN/EINTR
Message-ID: <459E4270.9090307@shadowen.org>
In-Reply-To: <7vvejlg1pg.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Andy Whitcroft <apw@shadowen.org> writes:
> 
>>     We have an xread() wrapper to help us with those nasty
>>     interrupt returns and yet we fail to use it consistently.
>>     This patch updates those plain read()'s which do not
>>     have any handling for errors, or which treat those errors
>>     as user visible fatal errors.
>>
>>     This feels right to me, but perhaps there is some good
>>     reason that things are done this way ... if so could
>>     someone elighten me.
> 
> Thanks.
> 
> I do not think any of the changes you did introduced new bugs,
> but I think some of them are still wrong.  xread() protects us
> from EINTR happening before any byte is read, but it can still
> give a short read.  Many callers have a loop like this:
> 
> 	do {
>         	size = xread(...);
>                 yet_to_go -= size;
> 	} while (yet_to_go);
> 
> but some are not (e.g. add_excludes_from_file_1() in dir.c
> expects xread() does not return before reading full buffer).

Yes, that is true.  I was going to fix that in the next step with the
writes.  But yes thats likely to involve them becoming 'read_in_full'
style thing and in fact churn us more.

Ignore this one and I'll look to do it 'right'.

-apw