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

[Riku Voipio]

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 <address@hidden>
> 
> ---
>  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) {
>