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

[Anthony Liguori <anthony@codemonkey.ws>]

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.

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

Regards,

Anthony Liguori

> I added these to osdep.c, and there are many calls in the block
> drivers, so osdep.o needs to be in BLOCK_OBJS as I do in my previous
> patch (getting rid of the duplicate definitions of qemu_malloc &c).
>
> The patch then uses these new functions whereever appropriate.  I
> think I have got each calling point correct but for obvious reasons I
> haven't done a thorough test.
>
> The resulting code is I think both smaller and more correct.  In most
> cases the correct behaviour was obvious.
>
> There was one nonobvious case: I removed unix_write from vl.c and
> replaced calls to it with calls to qemu_write.  unix_write looped on
> EAGAIN (though qemu_write doesn't) but I think this is wrong since
> that simply results in spinning if the fd is nonblocking and the write
> cannot complete immediately.  Callers with nonblocking fds have to
> cope with partial results and retry later.  Since unix_write doesn't
> do that I assume that its callers don't really have nonblocking fds;
> if they do then the old code is buggy and my new code is buggy too but
> in a different way.
>
> Also, the Makefile rule for dyngen$(EXESUF) referred to dyngen.c
> rather than dyngen.o, which appears to have been a mistake which I
> have fixed since I had to add osdep.o anyway.
>
> Regards,
> Ian.
>
>
>