On Thu, Jan 17, 2013 at 08:55:14AM +0100, Michal Privoznik wrote:

Currently, whenever somebody calls saferead() on nonblocking FD (safewrite() is totally interchangeable for purpose of this message) he might get wrong return value. For instance, in the first iteration some data is read. The number of bytes read is stored into local variable 'nread'. However, in next iterations we can get -1 from read() with errno == EAGAIN, in which case the -1 is returned despite fact some data has already been read. So the caller gets confused. Bare read() should be used for nonblocking FD. --- src/util/virutil.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c index b36e9d3..bc970a4 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -98,7 +98,9 @@ verify(sizeof(gid_t) <= sizeof(unsigned int) &&

#define VIR_FROM_THIS VIR_FROM_NONE

-/* Like read(), but restarts after EINTR */ +/* Like read(), but restarts after EINTR. + * Doesn't play nicely with nonblocking FD and EAGAIN, + * in which case you want to use bare read() */ ssize_t saferead(int fd, void *buf, size_t count) { @@ -118,7 +120,9 @@ saferead(int fd, void *buf, size_t count) return nread; }

-/* Like write(), but restarts after EINTR */ +/* Like write(), but restarts after EINTR. + * Doesn't play nicely with nonblocking FD and EAGAIN, + * in which case you want to use bare write() */ ssize_t safewrite(int fd, const void *buf, size_t count) {

ACK Or even use virSocket() if the FD is related to a socket rather than a plain file or pipe. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|