qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v4] avoid compilation warning/errors on up to da


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH v4] avoid compilation warning/errors on up to date
Date: Wed, 17 Jun 2009 16:54:56 -0500
User-agent: Thunderbird 2.0.0.21 (X11/20090320)

Jean-Christophe Dubois wrote:
Some system calls are now requiring to have their return value checked.

Without this a warning is emitted and in the case a qemu an error is
triggered as qemu is considering warnings as errors.

For example:

block/cow.c: In function ‘cow_create’:
block/cow.c:251: error: ignoring return value of ‘write’, declared with
attribute warn_unused_result
block/cow.c:253: error: ignoring return value of ‘ftruncate’, declared
with attribute warn_unused_result

This is an attempt at removing all these warnings to allow a clean
compilation with up to date compilers/distributions.

The second version fixes an error detected by Stuart Brady as well
as some coding style issues. Note however that some of the
modified files don't follow the qemu coding style (using tabs
instead of spaces).

The Third version add one ftruncate() system call error handling that
was missing from V2 (in block/vvfat.c).

The Fourth version is correctly handling EINTR error on read/write
system calls. read/write calls are replaced by qemu_read/qemu_write
functions that are handling EINTR and incomplete read/write under
the cover.

Signed-off-by: Jean-Christophe DUBOIS <address@hidden>

This really makes me nervous to be honest.  It's too much for one patch.

What I'd rather do, is commit Gerd's patch and also add -U_FORTIFY_SOURCE to the default CFLAGS. I've confirmed this fixes the default build on Ubuntu.

I'd rather take this set of clean ups as a set of smaller, well thought-out patches. Once that's done, we can remove -U_FORTIFY_SOURCE.

---
 block.c           |    3 +-
 block/bochs.c     |    3 +-
 block/cow.c       |   12 +++++++++-
 block/qcow.c      |   22 +++++++++++++++-----
 block/qcow2.c     |   37 ++++++++++++++++++++++++++++-------
 block/raw-posix.c |    9 +++++--
 block/vmdk.c      |   54 +++++++++++++++++++++++++++++++++++-----------------
 block/vvfat.c     |   24 ++++++++++++++++------
 linux-user/mmap.c |    7 ++++-
 linux-user/path.c |    6 ++++-
 osdep.c           |    5 +++-
 qemu-common.h     |   31 ++++++++++++++++++++++++++++++
 slirp/misc.c      |    4 ++-
 usb-linux.c       |    3 +-
 vl.c              |   14 +++++++++---
 15 files changed, 177 insertions(+), 57 deletions(-)

diff --git a/block.c b/block.c
index aca5a6d..c78d66a 100644
--- a/block.c
+++ b/block.c
@@ -371,7 +371,8 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
             snprintf(backing_filename, sizeof(backing_filename),
                      "%s", filename);
         else
-            realpath(filename, backing_filename);
+            if (!realpath(filename, backing_filename))
+                return -1;

There should be cleanup work to do, no?

bdrv_qcow2 = bdrv_find_format("qcow2");
         options = parse_option_parameters("", bdrv_qcow2->create_options, 
NULL);
diff --git a/block/bochs.c b/block/bochs.c
index bac81c4..1fbce9e 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -199,7 +199,8 @@ static inline int seek_to_sector(BlockDriverState *bs, 
int64_t sector_num)
     // read in bitmap for current extent
     lseek(s->fd, bitmap_offset + (extent_offset / 8), SEEK_SET);
- read(s->fd, &bitmap_entry, 1);
+    if (qemu_read(s->fd, &bitmap_entry, 1) != 1)
+        return -1; // not allocated

Why check errors on read but not lseek? If we're going to add error checking to this function, we should make it robust, not just enough error checking to silence GCC. We should just silence GCC with -U_FORTIFY_SOURCE.

The same is true for most of the remainder. This is definitely a good task worth doing but I think we should do it more carefully.

Regards,

Anthony Liguori




reply via email to

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