[Qemu-devel] [PATCH v4] avoid compilation warning/errors on up to date

* [Qemu-devel] [PATCH v4] avoid compilation warning/errors on up to date
@ 2009-06-17 21:22 Jean-Christophe Dubois
  2009-06-17 21:51 ` malc
  2009-06-17 21:54 ` Anthony Liguori
  0 siblings, 2 replies; 12+ messages in thread
From: Jean-Christophe Dubois @ 2009-06-17 21:22 UTC (permalink / raw)
  To: qemu-devel

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 <jcd@tribudubois.net>
---
 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;
 
         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
 
     if (!((bitmap_entry >> (extent_offset % 8)) & 1))
     {
diff --git a/block/cow.c b/block/cow.c
index 84818f1..d8cd1df 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -248,11 +248,19 @@ static int cow_create(const char *filename, QEMUOptionParameter *options)
     }
     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 (qemu_write(cow_fd, &cow_header, sizeof(cow_header)) != sizeof(cow_header))
+        goto fail;
+
     /* 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)))
+        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 55a68a6..5e50db8 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -801,17 +801,27 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options)
     }
 
     /* write all the data */
-    write(fd, &header, sizeof(header));
-    if (backing_file) {
-        write(fd, backing_file, backing_filename_len);
-    }
-    lseek(fd, header_size, SEEK_SET);
+    if (qemu_write(fd, &header, sizeof(header)) != sizeof(header))
+        goto fail;
+
+    if (backing_file)
+        if (qemu_write(fd, backing_file, backing_filename_len) != backing_filename_len)
+            goto fail;
+
+    if (lseek(fd, header_size, SEEK_SET) == -1)
+        goto fail;
     tmp = 0;
     for(i = 0;i < l1_size; i++) {
-        write(fd, &tmp, sizeof(tmp));
+        if (qemu_write(fd, &tmp, sizeof(tmp)) != sizeof(tmp))
+            goto fail;
     }
+
     close(fd);
     return 0;
+
+fail:
+    close(fd);
+    return -1;
 }
 
 static int qcow_make_empty(BlockDriverState *bs)
diff --git a/block/qcow2.c b/block/qcow2.c
index 9acbddf..fab3c7c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -742,7 +742,9 @@ static int qcow_create2(const char *filename, int64_t total_size,
         ref_clusters * s->cluster_size);
 
     /* write all the data */
-    write(fd, &header, sizeof(header));
+    if (qemu_write(fd, &header, sizeof(header)) != sizeof(header))
+        goto fail;
+
     if (backing_file) {
         if (backing_format_len) {
             char zero[16];
@@ -751,29 +753,48 @@ static int qcow_create2(const char *filename, int64_t total_size,
             memset(zero, 0, sizeof(zero));
             cpu_to_be32s(&ext_bf.magic);
             cpu_to_be32s(&ext_bf.len);
-            write(fd, &ext_bf, sizeof(ext_bf));
-            write(fd, backing_format, backing_format_len);
+            if (qemu_write(fd, &ext_bf, sizeof(ext_bf)) != sizeof(ext_bf))
+                goto fail;
+
+            if (qemu_write(fd, backing_format, backing_format_len) != backing_format_len)
+                goto fail;
+
             if (d>0) {
-                write(fd, zero, d);
+                if (qemu_write(fd, zero, d) != d)
+                    goto fail;
             }
         }
-        write(fd, backing_file, backing_filename_len);
+        if (qemu_write(fd, backing_file, backing_filename_len) != backing_filename_len)
+            goto fail;
     }
     lseek(fd, s->l1_table_offset, SEEK_SET);
     tmp = 0;
     for(i = 0;i < l1_size; i++) {
-        write(fd, &tmp, sizeof(tmp));
+        if (qemu_write(fd, &tmp, sizeof(tmp)) != sizeof(tmp))
+            goto fail;
     }
     lseek(fd, s->refcount_table_offset, SEEK_SET);
-    write(fd, s->refcount_table, s->cluster_size);
+    if (qemu_write(fd, s->refcount_table, s->cluster_size) != s->cluster_size)
+        goto fail;
 
     lseek(fd, s->refcount_block_offset, SEEK_SET);
-    write(fd, s->refcount_block, ref_clusters * s->cluster_size);
+    if (qemu_write(fd, s->refcount_block, ref_clusters * s->cluster_size) != ref_clusters * s->cluster_size)
+        goto fail;
 
     qemu_free(s->refcount_table);
+    s->refcount_table = NULL;
     qemu_free(s->refcount_block);
+    s->refcount_block = NULL;
     close(fd);
     return 0;
+
+fail:
+    qemu_free(s->refcount_table);
+    s->refcount_table = NULL;
+    qemu_free(s->refcount_block);
+    s->refcount_block = NULL;
+    close(fd);
+    return -1;
 }
 
 static int qcow_create(const char *filename, QEMUOptionParameter *options)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index fa1a394..47e0d15 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -558,7 +558,8 @@ static void aio_signal_handler(int signum)
     if (posix_aio_state) {
         char byte = 0;
 
-        write(posix_aio_state->wfd, &byte, sizeof(byte));
+        if (qemu_write(posix_aio_state->wfd, &byte, sizeof(byte)) != sizeof(byte))
+            fprintf(stderr, "failed to write to posix_aio_state\n");
     }
 
     qemu_service_io();
@@ -838,6 +839,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
 {
     int fd;
     int64_t total_size = 0;
+    int ret = 0;
 
     /* Read out options */
     while (options && options->name) {
@@ -851,9 +853,10 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
               0644);
     if (fd < 0)
         return -EIO;
-    ftruncate(fd, total_size * 512);
+    if (ftruncate(fd, total_size * 512))
+        ret = -1;
     close(fd);
-    return 0;
+    return ret;
 }
 
 static void raw_flush(BlockDriverState *bs)
diff --git a/block/vmdk.c b/block/vmdk.c
index f21f02b..b53b047 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -221,23 +221,24 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
     /* read the header */
     if (lseek(p_fd, 0x0, SEEK_SET) == -1)
         goto fail;
-    if (read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE)
+    if (qemu_read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE)
         goto fail;
 
     /* write the header */
     if (lseek(snp_fd, 0x0, SEEK_SET) == -1)
         goto fail;
-    if (write(snp_fd, hdr, HEADER_SIZE) == -1)
+    if (qemu_write(snp_fd, hdr, HEADER_SIZE) == -1)
         goto fail;
 
     memset(&header, 0, sizeof(header));
     memcpy(&header,&hdr[4], sizeof(header)); // skip the VMDK4_MAGIC
 
-    ftruncate(snp_fd, header.grain_offset << 9);
+    if (ftruncate(snp_fd, header.grain_offset << 9))
+        goto fail;
     /* the descriptor offset = 0x200 */
     if (lseek(p_fd, 0x200, SEEK_SET) == -1)
         goto fail;
-    if (read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE)
+    if (qemu_read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE)
         goto fail;
 
     if ((p_name = strstr(p_desc,"CID")) != NULL) {
@@ -259,7 +260,7 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
     /* write the descriptor */
     if (lseek(snp_fd, 0x200, SEEK_SET) == -1)
         goto fail;
-    if (write(snp_fd, s_desc, strlen(s_desc)) == -1)
+    if (qemu_write(snp_fd, s_desc, strlen(s_desc)) == -1)
         goto fail;
 
     gd_offset = header.gd_offset * SECTOR_SIZE;     // offset of GD table
@@ -279,11 +280,11 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
     rgd_buf = qemu_malloc(gd_size);
     if (lseek(p_fd, rgd_offset, SEEK_SET) == -1)
         goto fail_rgd;
-    if (read(p_fd, rgd_buf, gd_size) != gd_size)
+    if (qemu_read(p_fd, rgd_buf, gd_size) != gd_size)
         goto fail_rgd;
     if (lseek(snp_fd, rgd_offset, SEEK_SET) == -1)
         goto fail_rgd;
-    if (write(snp_fd, rgd_buf, gd_size) == -1)
+    if (qemu_write(snp_fd, rgd_buf, gd_size) == -1)
         goto fail_rgd;
     qemu_free(rgd_buf);
 
@@ -291,11 +292,11 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
     gd_buf = qemu_malloc(gd_size);
     if (lseek(p_fd, gd_offset, SEEK_SET) == -1)
         goto fail_gd;
-    if (read(p_fd, gd_buf, gd_size) != gd_size)
+    if (qemu_read(p_fd, gd_buf, gd_size) != gd_size)
         goto fail_gd;
     if (lseek(snp_fd, gd_offset, SEEK_SET) == -1)
         goto fail_gd;
-    if (write(snp_fd, gd_buf, gd_size) == -1)
+    if (qemu_write(snp_fd, gd_buf, gd_size) == -1)
         goto fail_gd;
     qemu_free(gd_buf);
 
@@ -771,22 +772,32 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
     header.check_bytes[3] = 0xa;
 
     /* write all the data */
-    write(fd, &magic, sizeof(magic));
-    write(fd, &header, sizeof(header));
+    if (qemu_write(fd, &magic, sizeof(magic)) != sizeof(magic))
+        goto fail;
 
-    ftruncate(fd, header.grain_offset << 9);
+    if (qemu_write(fd, &header, sizeof(header)) != sizeof(header))
+        goto fail;
+
+    if (ftruncate(fd, header.grain_offset << 9))
+        goto fail;
 
     /* write grain directory */
-    lseek(fd, le64_to_cpu(header.rgd_offset) << 9, SEEK_SET);
+    if (lseek(fd, le64_to_cpu(header.rgd_offset) << 9, SEEK_SET) == -1)
+        goto fail;
+
     for (i = 0, tmp = header.rgd_offset + gd_size;
          i < gt_count; i++, tmp += gt_size)
-        write(fd, &tmp, sizeof(tmp));
+        if (qemu_write(fd, &tmp, sizeof(tmp)) != sizeof(tmp))
+            goto fail;
 
     /* write backup grain directory */
-    lseek(fd, le64_to_cpu(header.gd_offset) << 9, SEEK_SET);
+    if (lseek(fd, le64_to_cpu(header.gd_offset) << 9, SEEK_SET) == -1)
+        goto fail;
+
     for (i = 0, tmp = header.gd_offset + gd_size;
          i < gt_count; i++, tmp += gt_size)
-        write(fd, &tmp, sizeof(tmp));
+        if (qemu_write(fd, &tmp, sizeof(tmp)) != sizeof(tmp))
+            goto fail;
 
     /* compose the descriptor */
     real_filename = filename;
@@ -802,11 +813,18 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
              total_size / (int64_t)(63 * 16));
 
     /* write the descriptor */
-    lseek(fd, le64_to_cpu(header.desc_offset) << 9, SEEK_SET);
-    write(fd, desc, strlen(desc));
+    if (lseek(fd, le64_to_cpu(header.desc_offset) << 9, SEEK_SET) == -1)
+        goto fail;
+
+    if (qemu_write(fd, desc, strlen(desc)) != strlen(desc))
+        goto fail;
 
     close(fd);
     return 0;
+
+fail:
+    close(fd);
+    return -1;
 }
 
 static void vmdk_close(BlockDriverState *bs)
diff --git a/block/vvfat.c b/block/vvfat.c
index 1e37b9f..097c9f6 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2215,6 +2215,7 @@ static int commit_one_file(BDRVVVFATState* s,
     char* cluster = qemu_malloc(s->cluster_size);
     uint32_t i;
     int fd = 0;
+    int ret = 0;
 
     assert(offset < size);
     assert((offset % s->cluster_size) == 0);
@@ -2229,14 +2230,15 @@ static int commit_one_file(BDRVVVFATState* s,
 	return fd;
     }
     if (offset > 0)
-	if (lseek(fd, offset, SEEK_SET) != offset)
-	    return -3;
+        if (lseek(fd, offset, SEEK_SET) != offset) {
+            ret = -3;
+            goto fail;
+        }
 
     while (offset < size) {
 	uint32_t c1;
 	int rest_size = (size - offset > s->cluster_size ?
 		s->cluster_size : size - offset);
-	int ret;
 
 	c1 = modified_fat_get(s, c);
 
@@ -2247,19 +2249,27 @@ static int commit_one_file(BDRVVVFATState* s,
 	    (uint8_t*)cluster, (rest_size + 0x1ff) / 0x200);
 
 	if (ret < 0)
-	    return ret;
+	    goto fail;
 
-	if (write(fd, cluster, rest_size) < 0)
-	    return -2;
+        if (qemu_write(fd, cluster, rest_size) != rest_size) {
+            ret = -2;
+            goto fail;
+        }
 
 	offset += rest_size;
 	c = c1;
     }
 
-    ftruncate(fd, size);
+    if (ftruncate(fd, size))
+        goto fail;
+
     close(fd);
 
     return commit_mappings(s, first_cluster, dir_index);
+
+fail:
+    close(fd);
+    return ret;
 }
 
 #ifdef DEBUG
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index aa22006..5a1b525 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -252,7 +252,8 @@ static int mmap_frag(abi_ulong real_start,
             mprotect(host_start, qemu_host_page_size, prot1 | PROT_WRITE);
 
         /* read the corresponding file data */
-        pread(fd, g2h(start), end - start, offset);
+        if (pread(fd, g2h(start), end - start, offset) == -1)
+            return -1;
 
         /* put final protection */
         if (prot_new != (prot1 | PROT_WRITE))
@@ -469,7 +470,9 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
                                   -1, 0);
             if (retaddr == -1)
                 goto fail;
-            pread(fd, g2h(start), len, offset);
+            if (pread(fd, g2h(start), len, offset) == -1)
+                goto fail;
+                
             if (!(prot & PROT_WRITE)) {
                 ret = target_mprotect(start, len, prot);
                 if (ret != 0) {
diff --git a/linux-user/path.c b/linux-user/path.c
index 06b1f5f..fc5cd6e 100644
--- a/linux-user/path.c
+++ b/linux-user/path.c
@@ -45,8 +45,12 @@ static struct pathelem *new_entry(const char *root,
 {
     struct pathelem *new = malloc(sizeof(*new));
     new->name = strdup(name);
-    asprintf(&new->pathname, "%s/%s", root, name);
     new->num_entries = 0;
+    if (asprintf(&new->pathname, "%s/%s", root, name) == -1) {
+        free(new->name);
+        free(new);
+        new = NULL;
+    }
     return new;
 }
 
diff --git a/osdep.c b/osdep.c
index b300ba1..de0124e 100644
--- a/osdep.c
+++ b/osdep.c
@@ -160,7 +160,10 @@ static void *kqemu_vmalloc(size_t size)
         unlink(phys_ram_file);
     }
     size = (size + 4095) & ~4095;
-    ftruncate(phys_ram_fd, phys_ram_size + size);
+    if (ftruncate(phys_ram_fd, phys_ram_size + size)) {
+        fprintf(stderr, "Could not truncate phys_ram_file\n");
+        exit(1);
+    }
 #endif /* !(__OpenBSD__ || __FreeBSD__ || __DragonFly__) */
     ptr = mmap(NULL,
                size,
diff --git a/qemu-common.h b/qemu-common.h
index fdc3679..4b75018 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -136,6 +136,37 @@ char *qemu_strndup(const char *str, size_t size);
 
 void *get_mmap_addr(unsigned long size);
 
+static inline ssize_t qemu_read(int fd, void *buf, size_t count)
+{
+   int len = 0;
+
+   while (len < count) {
+       int tmp = read(fd, buf + len, count - len);
+
+       if (tmp > 0)
+           len += tmp;
+       else if (errno != EINTR)
+           return -1;
+   }
+
+   return count;
+}
+
+static inline ssize_t qemu_write(int fd, const void *buf, size_t count)
+{
+   int len = 0;
+
+   while (len < count) {
+       int tmp = write(fd, buf + len, count - len);
+
+       if (tmp > 0)
+           len += tmp;
+       else if (errno != EINTR)
+           return -1;
+   }
+
+   return count;
+}
 
 /* Error handling.  */
 
diff --git a/slirp/misc.c b/slirp/misc.c
index 1391d49..46d5a83 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -6,6 +6,7 @@
  */
 
 #include <slirp.h>
+#include <qemu-common.h>
 
 u_int curtime, time_fasttimo, last_slowtimo;
 
@@ -365,7 +366,8 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
 			  snprintf(buff, sizeof(buff),
                                    "Error: execvp of %s failed: %s\n",
                                    argv[0], strerror(errno));
-			  write(2, buff, strlen(buff)+1);
+                          if (qemu_write(2, buff, strlen(buff)+1) != (strlen(buff)+1))
+                              lprint("Error: failed to write to stderr: %s\n", strerror(errno));
 		  }
 		close(0); close(1); close(2); /* XXX */
 		exit(1);
diff --git a/usb-linux.c b/usb-linux.c
index 67e4acd..d71c4b5 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1159,9 +1159,8 @@ static int usb_host_read_file(char *line, size_t line_size, const char *device_f
              device_file);
     f = fopen(filename, "r");
     if (f) {
-        fgets(line, line_size, f);
+        if (fgets(line, line_size, f)) ret = 1;
         fclose(f);
-        ret = 1;
     } else {
         monitor_printf(mon, "husb: could not open %s\n", filename);
     }
diff --git a/vl.c b/vl.c
index 3242c23..5e2c314 100644
--- a/vl.c
+++ b/vl.c
@@ -3707,7 +3707,8 @@ static void qemu_event_increment(void)
     if (io_thread_fd == -1)
         return;
 
-    write(io_thread_fd, &byte, sizeof(byte));
+    if (qemu_write(io_thread_fd, &byte, sizeof(byte)) != sizeof(byte))
+        perror("Failed write");
 }
 
 static void qemu_event_read(void *opaque)
@@ -5785,7 +5786,8 @@ int main(int argc, char **argv, char **envp)
     if (pid_file && qemu_create_pidfile(pid_file) != 0) {
         if (daemonize) {
             uint8_t status = 1;
-            write(fds[1], &status, 1);
+            if (qemu_write(fds[1], &status, 1) != 1)
+                fprintf(stderr, "Could not write status to pid file \n");
         } else
             fprintf(stderr, "Could not acquire pid file\n");
         exit(1);
@@ -6216,7 +6218,9 @@ int main(int argc, char **argv, char **envp)
 	if (len != 1)
 	    exit(1);
 
-	chdir("/");
+        if (chdir("/"))
+            exit(1);
+
 	TFR(fd = open("/dev/null", O_RDWR));
 	if (fd == -1)
 	    exit(1);
@@ -6235,7 +6239,9 @@ int main(int argc, char **argv, char **envp)
             fprintf(stderr, "chroot failed\n");
             exit(1);
         }
-        chdir("/");
+
+        if (chdir("/"))
+            exit(1);
     }
 
     if (run_as) {

^ permalink raw reply related	[flat|nested] 12+ messages in thread