From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M3Y9z-0004nb-IB for qemu-devel@nongnu.org; Mon, 11 May 2009 12:15:23 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M3Y9v-0004lM-UP for qemu-devel@nongnu.org; Mon, 11 May 2009 12:15:23 -0400 Received: from [199.232.76.173] (port=47765 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M3Y9v-0004lG-QC for qemu-devel@nongnu.org; Mon, 11 May 2009 12:15:19 -0400 Received: from mx1.redhat.com ([66.187.233.31]:49514) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1M3Y9v-00067F-Ft for qemu-devel@nongnu.org; Mon, 11 May 2009 12:15:19 -0400 Date: Mon, 11 May 2009 17:15:15 +0100 From: "Daniel P. Berrange" Subject: Re: [Qemu-devel] [PATCH] suppress 'warn_unused_result' warning Message-ID: <20090511161515.GA7858@redhat.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Reply-To: "Daniel P. Berrange" List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chih-Min Chao Cc: qemu-devel@nongnu.org On Mon, May 11, 2009 at 03:15:11AM +0800, Chih-Min Chao wrote: > The patch add error handling to functions with 'warn_unused_result' return > value > such as write, read, ftruncate, and realpath. > @@ -239,11 +239,18 @@ static int cow_create(const char *filename, int64_t image_sectors, > } > cow_header.sectorsize = cpu_to_be32(512); > cow_header.size = cpu_to_be64(image_sectors * 512); > - write(cow_fd, &cow_header, sizeof(cow_header)); > + if (write(cow_fd, &cow_header, sizeof(cow_header)) == -1) > + goto fail; This isn't correct. You need to check that the write() actually wrote the number of bytes you asked it to., eg if (write(cow_fd, &cow_header, sizeof(cow_header)) != sizeof(cow_header) goto fail; would catch a short write, as well as other errors. Of course you don't neccessarily want to fail on a short write, because a reception of a signal can trigger a short write that can easily be recovered from by simply calling write() against for the remainder of the data. > /* resize to include at least all the bitmap */ > - ftruncate(cow_fd, sizeof(cow_header) + ((image_sectors + 7) >> 3)); > + if (ftruncate(cow_fd, sizeof(cow_header) + ((image_sectors + 7) >> 3)) == -1) > + goto fail; > + > close(cow_fd); > return 0; > + > +fail: > + close(cow_fd); > + return -1; > } > > static void cow_flush(BlockDriverState *bs) > diff --git a/block-qcow.c b/block-qcow.c > index fc6b809..16138f3 100644 > --- a/block-qcow.c > +++ b/block-qcow.c > @@ -811,17 +811,28 @@ static int qcow_create(const char *filename, int64_t total_size, > } > > /* write all the data */ > - write(fd, &header, sizeof(header)); > + if (write(fd, &header, sizeof(header)) == -1) { > + goto fail; > + } > + > if (backing_file) { > - write(fd, backing_file, backing_filename_len); > + if (write(fd, backing_file, backing_filename_len) == -1) { > + goto fail; > + } > } > lseek(fd, header_size, SEEK_SET); > tmp = 0; > for(i = 0;i < l1_size; i++) { > - write(fd, &tmp, sizeof(tmp)); > + if (write(fd, &tmp, sizeof(tmp)) == -1) { > + goto fail; > + } > } Likewise all these are failing to check for a complete write. If we want to make this robust for EINTR too, then a small wrapper around raw read/write calls would likely be wanted to deal with fact an a signal can cause EINTR, *or* a short write ssize_t safewrite(int fd, const void *buf, size_t count) { size_t nwritten = 0; while (count > 0) { ssize_t r = write(fd, buf, count); if (r < 0 && errno == EINTR) continue; if (r < 0) return r; if (r == 0) return nwritten; buf = (const char *)buf + r; count -= r; nwritten += r; } return nwritten; } Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|