Re: [Qemu-devel] [PATCH] check return value from read() and write() properly

[Ian Jackson]

Anthony Liguori writes ("Re: [Qemu-devel] [PATCH] check return value from 
read() and write() properly"):
> Ian Jackson wrote:
> > The system calls read and write may return less than the whole amount
> > requested for a number of reasons.  So the idioms
> >    if (read(fd, &object, sizeof(object)) != sizeof(object)) goto fail;
> > and even worse
> >    if (read(fd, &object, sizeof(object)) < 0) goto fail;
> > are wrong.  Additionally, read and write may sometimes return EINTR on
> > some systems so interruption is not desired or expected a loop is
> > needed.
> >
> > In the attached patch I introduce two new pairs of functions:
> >  qemu_{read,write}      which are like read and write but never
> >                           return partial answers unnecessarily
> >                           and which never fail with EINTR
> >  qemu_{read,write}_ok   which returns -1 for any failure or
> >                           incomplete read, or +1 for success,
> >                           reducing repetition at calling points
> 
> There is already a unix_write function that serves this purpose.  I 
> think a better approach would be to define unix_read/unix_write and 
> remove the EAGAIN handling and instead only spin on EINTR.

Is this just an argument about the name ?  I chose names like
qemu_read and qemu_write by analogy with qemu_malloc.  unix_write's
name is a bit of an anomaly.  It is better to call it qemu_write
because that makes it clearer that these functions should be used
pretty much everywhere.

Also, unix_write is in the wrong file.  It has to be in osdep.c so
that programs like qemu-img pick it up.

> I don't really like the _ok thing as it's not a very common idiom.

Picking a random example from the code (loader.c near line 50):

  -    if (read(fd, addr, size) != size) {
  +    if (qemu_read_ok(fd, addr, size) < 0) {

This is an improvement because you only have to write `size' once.
The idiom being replaced above is very common in the existing code and
is very clumsy when we get to things like this (dyngen.c near line
1130, indent removed to make it more readable):

- if(read(fd, dysymtabcmd, sizeof(struct dysymtab_command)) != sizeof(struct 
dysymtab_command))
+ if(qemu_read_ok(fd, dysymtabcmd, sizeof(struct dysymtab_command)) < 0)

As I say the former idiom is common in qemu but it is cumbersome.
As ever, facilities should be provided - and used - to improve
cumbersome idioms.

Ian.