From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JMpn5-00077N-VK for qemu-devel@nongnu.org; Wed, 06 Feb 2008 14:18:40 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JMpn4-00076r-9Z for qemu-devel@nongnu.org; Wed, 06 Feb 2008 14:18:39 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JMpn4-00076o-6q for qemu-devel@nongnu.org; Wed, 06 Feb 2008 14:18:38 -0500 Received: from wx-out-0506.google.com ([66.249.82.231]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1JMpn3-0000s2-Kh for qemu-devel@nongnu.org; Wed, 06 Feb 2008 14:18:37 -0500 Received: by wx-out-0506.google.com with SMTP id h31so3410455wxd.4 for ; Wed, 06 Feb 2008 11:18:36 -0800 (PST) Message-ID: <47AA0807.40707@codemonkey.ws> Date: Wed, 06 Feb 2008 13:18:31 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] check return value from read() and write() properly References: <18329.60886.909252.289862@mariner.uk.xensource.com> In-Reply-To: <18329.60886.909252.289862@mariner.uk.xensource.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org 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. > > >