From: Riku Voipio <riku.voipio@iki.fi>
To: Jean-Christophe Dubois <jcd@tribudubois.net>
Cc: qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH] avoid compilation warning/errors on up to date compilers
Date: Tue, 16 Jun 2009 18:03:43 +0300 [thread overview]
Message-ID: <20090616150343.GA4350@kos.to> (raw)
In-Reply-To: <200906160530.37086.jcd@tribudubois.net>
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) {
>
next prev parent reply other threads:[~2009-06-16 15:03 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090616150343.GA4350@kos.to \
--to=riku.voipio@iki.fi \
--cc=jcd@tribudubois.net \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.