[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH] check return value from read() and write() properly
Date: Wed, 06 Feb 2008 13:18:31 -0600
User-agent: Thunderbird (X11/20071229)

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

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.


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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]