Re: [RFC 3/3] libbb: read_full: use read return instead size

[Alexander Aring <alex.aring@gmail.com>]

On Fri, Feb 28, 2014 at 06:12:07PM +0100, Alexander Aring wrote:
> Hi Sascha,
> 
> On Fri, Feb 28, 2014 at 03:21:18PM +0100, Sascha Hauer wrote:
> > On Fri, Feb 28, 2014 at 09:03:34AM +0100, Alexander Aring wrote:
> > > On Fri, Feb 28, 2014 at 08:44:28AM +0100, Alexander Aring wrote:
> > > > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > > > ---
> > > >  lib/libbb.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/lib/libbb.c b/lib/libbb.c
> > > > index 189a170..c8d0835 100644
> > > > --- a/lib/libbb.c
> > > > +++ b/lib/libbb.c
> > > > @@ -162,7 +162,7 @@ int read_full(int fd, void *buf, size_t size)
> > > >  	int now;
> > > >  	int total = 0;
> > > >  
> > > > -	while (size) {
> > > > +	while (now) {
> > > >  		now = read(fd, buf, size);
> > > >  		if (now == 0)
> > > >  			return total;
> > > and this should be a:
> > > 
> > > do {
> > > ...
> > > } while (now);
> > > 
> > > sry, it's only to demonstrate the issue.
> > 
> > 'now' will never be 0 at the end of the loop, so you could equally well
> > write while(1). With this change we try to read as long as we read
> yes, I did it quickly to make something that command "edit" works in
> some way. The whole patches is only to demonstrate the issue. Also the
> foofs demo works if we read the whole thing at once, but that's okay for
> the test.
> 
> 
> > something last time, even if there's nothing left to read (size is 0).
> > What issue do you see with this function?
> 
> The function read_full makes similar things like:
> 
> stat(fd, &statbuf);
> ...
> read(fd, buf, statbuf.st_size);
> 
> this is wrong because a filesize can be zero and read can read something
> from this file.
> 
> For example procfs in linux, if you run this under linux for /proc/version
> it will do nothing.
> 
> a "while (size)" don't call read because at first we checked if the file
> is zero, which don't indicate that you can read something from a file.
> 
I meant s/can read/can't read/

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox