All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] avoid compilation warning/errors on up to date compilers
@ 2009-06-16  3:30 Jean-Christophe Dubois
  2009-06-16 15:03 ` Riku Voipio
  0 siblings, 1 reply; 6+ messages in thread
From: Jean-Christophe Dubois @ 2009-06-16  3:30 UTC (permalink / raw)
  To: qemu-devel

Some system calls are now requiring to have their return value checked.

Without this a warning is emited 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.

Signed-off-by: Jean-Christophe DUBOIS <jcd@tribudubois.net>

---
 block.c           |    3 ++-
 block/bochs.c     |    3 ++-
 block/cow.c       |   14 +++++++++++---
 block/qcow.c      |   22 ++++++++++++++++------
 block/qcow2.c     |   37 +++++++++++++++++++++++++++++--------
 block/raw-posix.c |    9 ++++++---
 block/vmdk.c      |   38 ++++++++++++++++++++++++++++----------
 block/vvfat.c     |   26 +++++++++++++++++++-------
 linux-user/mmap.c |    7 +++++--
 linux-user/path.c |    6 +++++-
 osdep.c           |    5 ++++-
 qemu-nbd.c        |    3 ++-
 slirp/misc.c      |    3 ++-
 usb-linux.c       |    3 +--
 vl.c              |   12 ++++++++----
 15 files changed, 140 insertions(+), 51 deletions(-)

diff --git a/block.c b/block.c
index e6b91c6..28edf4d 100644
--- a/block.c
+++ b/block.c
@@ -368,7 +368,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..c80b86e 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 (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..0893ec1 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 (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..fc581ec 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 (write(fd, &header, sizeof(header)) != sizeof(header))
+        goto fail;
+
+    if (backing_file)
+        if (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 (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 c2be42e..7ee7e62 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1663,7 +1663,9 @@ static int qcow_create2(const char *filename, int64_t total_size,
     create_refcount_update(s, s->refcount_block_offset, ref_clusters * s->cluster_size);
 
     /* write all the data */
-    write(fd, &header, sizeof(header));
+    if (write(fd, &header, sizeof(header)) != sizeof(header))
+        goto fail;
+
     if (backing_file) {
         if (backing_format_len) {
             char zero[16];
@@ -1672,29 +1674,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 (write(fd, &ext_bf, sizeof(ext_bf)) != sizeof(ext_bf))
+                goto fail;
+
+            if (write(fd, backing_format, backing_format_len) != backing_format_len)
+                goto fail;
+
             if (d>0) {
-                write(fd, zero, d);
+                if (write(fd, zero, d) != d)
+                    goto fail;
             }
         }
-        write(fd, backing_file, backing_filename_len);
+        if (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 (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 (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 (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 4798d62..14119f1 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -552,7 +552,8 @@ static void aio_signal_handler(int signum)
     if (posix_aio_state) {
         char byte = 0;
 
-        write(posix_aio_state->wfd, &byte, sizeof(byte));
+        if (write(posix_aio_state->wfd, &byte, sizeof(byte)) != sizeof(byte))
+            fprintf(stderr, "failed to write to posix_aio_state\n");
     }
 
     qemu_service_io();
@@ -832,6 +833,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) {
@@ -845,9 +847,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..136d11b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -233,7 +233,8 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
     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;
@@ -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 (write(fd, &magic, sizeof(magic)) != sizeof(magic))
+        goto fail;
 
-    ftruncate(fd, header.grain_offset << 9);
+    if (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 (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 (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 (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..c4ac9b9 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,29 @@ 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 (write(fd, cluster, rest_size) < 0) {
+            ret = -2;
+            goto fail;
+        }
 
 	offset += rest_size;
 	c = c1;
     }
 
-    ftruncate(fd, size);
+    if (ftruncate(fd, size)) {
+        ret = -1;
+        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 6f300a0..a5b069e 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..f716122 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-nbd.c b/qemu-nbd.c
index 0af97ca..0a8d85f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -346,7 +346,8 @@ int main(int argc, char **argv)
         int sock;
 
         if (!verbose)
-            daemon(0, 0);	/* detach client and server */
+            if (daemon(0, 0))	/* detach client and server */
+                errx(errno, "Could not detach client and server");
 
         if (socket == NULL) {
             sprintf(sockpath, SOCKET_PATH, basename(device));
diff --git a/slirp/misc.c b/slirp/misc.c
index 1391d49..e09cb8a 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -365,7 +365,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 (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 bdd78cf..6750f9d 100644
--- a/vl.c
+++ b/vl.c
@@ -3691,7 +3691,8 @@ static void qemu_event_increment(void)
     if (io_thread_fd == -1)
         return;
 
-    write(io_thread_fd, &byte, sizeof(byte));
+    if (write(io_thread_fd, &byte, sizeof(byte)) != sizeof(byte))
+        perror("Failed write");
 }
 
 static void qemu_event_read(void *opaque)
@@ -5766,7 +5767,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 (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);
@@ -6203,7 +6205,8 @@ 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);
@@ -6222,7 +6225,8 @@ 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] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] avoid compilation warning/errors on up to date compilers
  2009-06-16  3:30 [Qemu-devel] [PATCH] avoid compilation warning/errors on up to date compilers Jean-Christophe Dubois
@ 2009-06-16 15:03 ` Riku Voipio
  2009-06-16 18:23   ` Stuart Brady
  0 siblings, 1 reply; 6+ messages in thread
From: Riku Voipio @ 2009-06-16 15:03 UTC (permalink / raw)
  To: Jean-Christophe Dubois; +Cc: qemu-devel

On Tue, Jun 16, 2009 at 05:30:36AM +0200, Jean-Christophe Dubois wrote:
> Some system calls are now requiring to have their return value checked.

> Without this a warning is emited 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.

Which tree against did you make these changes? the qemu-ndb.c bit didn't
apply against HEAD. Also, there are some whitespace/tab issues. See the
CODING_STYLE doc. Functionally I can verify it removes compiler warnings
when using a modenr glibc (2.9).

> Signed-off-by: Jean-Christophe DUBOIS <jcd@tribudubois.net>
> 
> ---
>  block.c           |    3 ++-
>  block/bochs.c     |    3 ++-
>  block/cow.c       |   14 +++++++++++---
>  block/qcow.c      |   22 ++++++++++++++++------
>  block/qcow2.c     |   37 +++++++++++++++++++++++++++++--------
>  block/raw-posix.c |    9 ++++++---
>  block/vmdk.c      |   38 ++++++++++++++++++++++++++++----------
>  block/vvfat.c     |   26 +++++++++++++++++++-------
>  linux-user/mmap.c |    7 +++++--
>  linux-user/path.c |    6 +++++-
>  osdep.c           |    5 ++++-
>  qemu-nbd.c        |    3 ++-
>  slirp/misc.c      |    3 ++-
>  usb-linux.c       |    3 +--
>  vl.c              |   12 ++++++++----
>  15 files changed, 140 insertions(+), 51 deletions(-)
> 
> diff --git a/block.c b/block.c
> index e6b91c6..28edf4d 100644
> --- a/block.c
> +++ b/block.c
> @@ -368,7 +368,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..c80b86e 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 (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..0893ec1 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 (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..fc581ec 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 (write(fd, &header, sizeof(header)) != sizeof(header))
> +        goto fail;
> +
> +    if (backing_file)
> +        if (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 (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 c2be42e..7ee7e62 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1663,7 +1663,9 @@ static int qcow_create2(const char *filename, int64_t total_size,
>      create_refcount_update(s, s->refcount_block_offset, ref_clusters * s->cluster_size);
>  
>      /* write all the data */
> -    write(fd, &header, sizeof(header));
> +    if (write(fd, &header, sizeof(header)) != sizeof(header))
> +        goto fail;
> +
>      if (backing_file) {
>          if (backing_format_len) {
>              char zero[16];
> @@ -1672,29 +1674,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 (write(fd, &ext_bf, sizeof(ext_bf)) != sizeof(ext_bf))
> +                goto fail;
> +
> +            if (write(fd, backing_format, backing_format_len) != backing_format_len)
> +                goto fail;
> +
>              if (d>0) {
> -                write(fd, zero, d);
> +                if (write(fd, zero, d) != d)
> +                    goto fail;
>              }
>          }
> -        write(fd, backing_file, backing_filename_len);
> +        if (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 (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 (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 (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 4798d62..14119f1 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -552,7 +552,8 @@ static void aio_signal_handler(int signum)
>      if (posix_aio_state) {
>          char byte = 0;
>  
> -        write(posix_aio_state->wfd, &byte, sizeof(byte));
> +        if (write(posix_aio_state->wfd, &byte, sizeof(byte)) != sizeof(byte))
> +            fprintf(stderr, "failed to write to posix_aio_state\n");
>      }
>  
>      qemu_service_io();
> @@ -832,6 +833,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) {
> @@ -845,9 +847,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..136d11b 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -233,7 +233,8 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
>      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;
> @@ -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 (write(fd, &magic, sizeof(magic)) != sizeof(magic))
> +        goto fail;
>  
> -    ftruncate(fd, header.grain_offset << 9);
> +    if (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 (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 (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 (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..c4ac9b9 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,29 @@ 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 (write(fd, cluster, rest_size) < 0) {
> +            ret = -2;
> +            goto fail;
> +        }
>  
>  	offset += rest_size;
>  	c = c1;
>      }
>  
> -    ftruncate(fd, size);
> +    if (ftruncate(fd, size)) {
> +        ret = -1;
> +        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 6f300a0..a5b069e 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..f716122 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-nbd.c b/qemu-nbd.c
> index 0af97ca..0a8d85f 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -346,7 +346,8 @@ int main(int argc, char **argv)
>          int sock;
>  
>          if (!verbose)
> -            daemon(0, 0);	/* detach client and server */
> +            if (daemon(0, 0))	/* detach client and server */
> +                errx(errno, "Could not detach client and server");
>  
>          if (socket == NULL) {
>              sprintf(sockpath, SOCKET_PATH, basename(device));
> diff --git a/slirp/misc.c b/slirp/misc.c
> index 1391d49..e09cb8a 100644
> --- a/slirp/misc.c
> +++ b/slirp/misc.c
> @@ -365,7 +365,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 (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 bdd78cf..6750f9d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3691,7 +3691,8 @@ static void qemu_event_increment(void)
>      if (io_thread_fd == -1)
>          return;
>  
> -    write(io_thread_fd, &byte, sizeof(byte));
> +    if (write(io_thread_fd, &byte, sizeof(byte)) != sizeof(byte))
> +        perror("Failed write");
>  }
>  
>  static void qemu_event_read(void *opaque)
> @@ -5766,7 +5767,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 (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);
> @@ -6203,7 +6205,8 @@ 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);
> @@ -6222,7 +6225,8 @@ 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	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] avoid compilation warning/errors on up to date compilers
  2009-06-16 15:03 ` Riku Voipio
@ 2009-06-16 18:23   ` Stuart Brady
  2009-06-16 20:01     ` Jean-Christophe Dubois
  2009-06-16 20:56     ` Anthony Liguori
  0 siblings, 2 replies; 6+ messages in thread
From: Stuart Brady @ 2009-06-16 18:23 UTC (permalink / raw)
  To: qemu-devel

On Tue, Jun 16, 2009 at 06:03:43PM +0300, Riku Voipio wrote:
> Which tree against did you make these changes? the qemu-ndb.c bit didn't
> apply against HEAD. Also, there are some whitespace/tab issues. See the
> CODING_STYLE doc. Functionally I can verify it removes compiler warnings
> when using a modenr glibc (2.9).

Unfortunately, some of the changes don't look right.

For instance:

> > -    read(s->fd, &bitmap_entry, 1);
> > +    if (read(s->fd, &bitmap_entry, 1) != 1)
> > +	return -1; // not allocated

If read() returns -1 and errno is set to EINTR, then the read() should
be reattempted.

> > -    write(cow_fd, &cow_header, sizeof(cow_header));
> > +    if (write(cow_fd, &cow_header, sizeof(cow_header)) == sizeof(cow_header))
> > +        goto fail;

Doesn't write(...) == sizeof(cow_header) indicate success?

Also, I gather partial reads/writes need to be supported.
-- 
Stuart Brady

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

* Re: [Qemu-devel] [PATCH] avoid compilation warning/errors on up to date compilers
  2009-06-16 18:23   ` Stuart Brady
@ 2009-06-16 20:01     ` Jean-Christophe Dubois
  2009-06-16 20:56     ` Anthony Liguori
  1 sibling, 0 replies; 6+ messages in thread
From: Jean-Christophe Dubois @ 2009-06-16 20:01 UTC (permalink / raw)
  To: qemu-devel

Le mardi 16 juin 2009 20:23:59 Stuart Brady, vous avez écrit :
> On Tue, Jun 16, 2009 at 06:03:43PM +0300, Riku Voipio wrote:
> > Which tree against did you make these changes? the qemu-ndb.c bit didn't
> > apply against HEAD.

It is whatever you get when cloning the qemu tree with "git clone 
git://git.savannah.nongnu.org/qemu.git" as proposed on the savannah web site.

> > Also, there are some whitespace/tab issues. See the
> > CODING_STYLE doc. Functionally I can verify it removes compiler warnings
> > when using a modenr glibc (2.9).

I'll have a look at the CODING_STYLE but I was under the impression I didn't 
use the tab stuff. I'll double check.

> Unfortunately, some of the changes don't look right.
>
> For instance:
> > > -    read(s->fd, &bitmap_entry, 1);
> > > +    if (read(s->fd, &bitmap_entry, 1) != 1)
> > > +	return -1; // not allocated
>
> If read() returns -1 and errno is set to EINTR, then the read() should
> be reattempted.

I can agree with this. However you will have to admit that even if this 
particular error case is not handled as it should be it is still a big 
improvement as there was no error handling at all previously. The main goal 
here is to allow that qemu compilation on up to date systems like Ubuntu 9.04 
(my case).

> > > -    write(cow_fd, &cow_header, sizeof(cow_header));
> > > +    if (write(cow_fd, &cow_header, sizeof(cow_header)) ==
> > > sizeof(cow_header)) +        goto fail;
>
> Doesn't write(...) == sizeof(cow_header) indicate success?

OOPS, my bad. I'll fix it.

> Also, I gather partial reads/writes need to be supported.

Once again, previously there was no error handling at all. What I propose is 
not perfect but at least it does some error handling and allow the qemu 
compilation on my system.

Supporting partial read/write would certainly require a lot more code 
refactoring.

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

* Re: [Qemu-devel] [PATCH] avoid compilation warning/errors on up to date compilers
  2009-06-16 18:23   ` Stuart Brady
  2009-06-16 20:01     ` Jean-Christophe Dubois
@ 2009-06-16 20:56     ` Anthony Liguori
  2009-06-16 21:07       ` Jean-Christophe Dubois
  1 sibling, 1 reply; 6+ messages in thread
From: Anthony Liguori @ 2009-06-16 20:56 UTC (permalink / raw)
  To: Stuart Brady; +Cc: qemu-devel

Stuart Brady wrote:
> On Tue, Jun 16, 2009 at 06:03:43PM +0300, Riku Voipio wrote:
>   
>> Which tree against did you make these changes? the qemu-ndb.c bit didn't
>> apply against HEAD. Also, there are some whitespace/tab issues. See the
>> CODING_STYLE doc. Functionally I can verify it removes compiler warnings
>> when using a modenr glibc (2.9).
>>     
>
> Unfortunately, some of the changes don't look right.
>   

I think adding -D_FORTIFY_SOURCE=1 to CFLAGS is probably a better fix 
than adding broken error handling everywhere.

Regards,

Anthony Liguori

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

* Re: [Qemu-devel] [PATCH] avoid compilation warning/errors on up to date compilers
  2009-06-16 20:56     ` Anthony Liguori
@ 2009-06-16 21:07       ` Jean-Christophe Dubois
  0 siblings, 0 replies; 6+ messages in thread
From: Jean-Christophe Dubois @ 2009-06-16 21:07 UTC (permalink / raw)
  To: qemu-devel

Le mardi 16 juin 2009 22:56:43 Anthony Liguori, vous avez écrit :
> Stuart Brady wrote:
> > On Tue, Jun 16, 2009 at 06:03:43PM +0300, Riku Voipio wrote:
> >> Which tree against did you make these changes? the qemu-ndb.c bit didn't
> >> apply against HEAD. Also, there are some whitespace/tab issues. See the
> >> CODING_STYLE doc. Functionally I can verify it removes compiler warnings
> >> when using a modenr glibc (2.9).
> >
> > Unfortunately, some of the changes don't look right.
>
> I think adding -D_FORTIFY_SOURCE=1 to CFLAGS is probably a better fix
> than adding broken error handling everywhere.

If the error handling I am proposing is not perfect at least it does something 
instead of being non existent.

Is it really considered as a better solution to just ignore what the people 
maintaining gcc/glibc consider should not be ignored?

JC

>
> Regards,
>
> Anthony Liguori

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

end of thread, other threads:[~2009-06-16 21:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-16  3:30 [Qemu-devel] [PATCH] avoid compilation warning/errors on up to date compilers Jean-Christophe Dubois
2009-06-16 15:03 ` Riku Voipio
2009-06-16 18:23   ` Stuart Brady
2009-06-16 20:01     ` Jean-Christophe Dubois
2009-06-16 20:56     ` Anthony Liguori
2009-06-16 21:07       ` Jean-Christophe Dubois

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.