[Qemu-devel] [PATCH 00/14] gcc extra warning fixes v2

[Qemu-devel] [PATCH 00/14] gcc extra warning fixes v2

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi,

I started building QEMU with some more aggressive error flags to see
what dropped out. I started fixing up some of the cases, removing
unused arguments to functions, comparisons of unsigned types against
negative values etc. and a few other minor changes to avoid compiler
warnings.

v2 fixes the vnc change as pointed out by Anthony.

Set of 14 patches following.

Cheers,
Jes

Jes Sorensen (14):
  Remove unused argument for nbd_client()
  Respect return value from nbd_client()
  Fix repeated typo: was "end if list" instead of "end of list"
  Zero initialize timespec struct explicitly
  Remove unused argument for check_for_block_signature()
  Remove unused argument for encrypt_sectors()
  Remove unused argument for get_whole_cluster()
  Remove unused argument for qcow2_encrypt_sectors()
  Remove unused arguments for add_aio_request() and free_aio_req()
  Zero json struct with memset() instea of = {} to keep compiler happy.
  Remove unused function arguments
  size_t is unsigned, change to ssize_t to handle errors from
    tight_compress_data()
  Change DPRINTF() to do{}while(0) to avoid compiler warning
  load_multiboot(): get_image_size() returns int

 block/qcow.c          |   13 ++++++-------
 block/qcow2-cluster.c |   17 ++++++++---------
 block/qcow2.c         |   10 +++++-----
 block/qcow2.h         |    7 +++----
 block/raw.c           |    4 ++--
 block/sheepdog.c      |   20 ++++++++++----------
 block/vmdk.c          |    4 ++--
 hw/multiboot.c        |    2 +-
 json-parser.c         |   39 +++++++++++++++++++--------------------
 linux-aio.c           |    2 +-
 nbd.c                 |    4 ++--
 nbd.h                 |    2 +-
 qemu-config.c         |   12 ++++++------
 qemu-nbd.c            |    5 ++++-
 qjson.c               |    3 ++-
 slirp/bootp.c         |    2 +-
 ui/vnc-enc-tight.c    |    8 ++++----
 17 files changed, 77 insertions(+), 77 deletions(-)

-- 
1.7.2.2

[Qemu-devel] [PATCH 01/14] Remove unused argument for nbd_client()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 nbd.c      |    4 ++--
 nbd.h      |    2 +-
 qemu-nbd.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/nbd.c b/nbd.c
index a9f295f..7049998 100644
--- a/nbd.c
+++ b/nbd.c
@@ -393,7 +393,7 @@ int nbd_disconnect(int fd)
 	return 0;
 }
 
-int nbd_client(int fd, int csock)
+int nbd_client(int fd)
 {
 	int ret;
 	int serrno;
@@ -427,7 +427,7 @@ int nbd_disconnect(int fd)
     return -1;
 }
 
-int nbd_client(int fd, int csock)
+int nbd_client(int fd)
 {
     errno = ENOTSUP;
     return -1;
diff --git a/nbd.h b/nbd.h
index 5a1fbdf..f103c3c 100644
--- a/nbd.h
+++ b/nbd.h
@@ -55,7 +55,7 @@ int nbd_send_request(int csock, struct nbd_request *request);
 int nbd_receive_reply(int csock, struct nbd_reply *reply);
 int nbd_trip(BlockDriverState *bs, int csock, off_t size, uint64_t dev_offset,
              off_t *offset, bool readonly, uint8_t *data, int data_size);
-int nbd_client(int fd, int csock);
+int nbd_client(int fd);
 int nbd_disconnect(int fd);
 
 #endif
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 4e607cf..9cc8f47 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -417,7 +417,7 @@ int main(int argc, char **argv)
 
             show_parts(device);
 
-            nbd_client(fd, sock);
+            nbd_client(fd);
             close(fd);
  out:
             kill(pid, SIGTERM);
-- 
1.7.2.2

[Qemu-devel] [PATCH 02/14] Respect return value from nbd_client()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 qemu-nbd.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 9cc8f47..67ce50b 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -417,7 +417,10 @@ int main(int argc, char **argv)
 
             show_parts(device);
 
-            nbd_client(fd);
+            ret = nbd_client(fd);
+            if (ret) {
+                ret = 1;
+            }
             close(fd);
  out:
             kill(pid, SIGTERM);
-- 
1.7.2.2

[Qemu-devel] [PATCH 03/14] Fix repeated typo: was "end if list" instead of "end of list"

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 qemu-config.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 3abe655..1efdbec 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -80,7 +80,7 @@ static QemuOptsList qemu_drive_opts = {
             .name = "readonly",
             .type = QEMU_OPT_BOOL,
         },
-        { /* end if list */ }
+        { /* end of list */ }
     },
 };
 
@@ -147,7 +147,7 @@ static QemuOptsList qemu_chardev_opts = {
             .name = "signal",
             .type = QEMU_OPT_BOOL,
         },
-        { /* end if list */ }
+        { /* end of list */ }
     },
 };
 
@@ -203,7 +203,7 @@ static QemuOptsList qemu_device_opts = {
          * sanity checking will happen later
          * when setting device properties
          */
-        { /* end if list */ }
+        { /* end of list */ }
     },
 };
 
@@ -247,7 +247,7 @@ static QemuOptsList qemu_rtc_opts = {
             .name = "driftfix",
             .type = QEMU_OPT_STRING,
         },
-        { /* end if list */ }
+        { /* end of list */ }
     },
 };
 
@@ -265,7 +265,7 @@ static QemuOptsList qemu_global_opts = {
             .name = "value",
             .type = QEMU_OPT_STRING,
         },
-        { /* end if list */ }
+        { /* end of list */ }
     },
 };
 
@@ -284,7 +284,7 @@ static QemuOptsList qemu_mon_opts = {
             .name = "default",
             .type = QEMU_OPT_BOOL,
         },
-        { /* end if list */ }
+        { /* end of list */ }
     },
 };
 
-- 
1.7.2.2

[Qemu-devel] [PATCH 04/14] Zero initialize timespec struct explicitly

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 linux-aio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-aio.c b/linux-aio.c
index 68f4b3d..3240996 100644
--- a/linux-aio.c
+++ b/linux-aio.c
@@ -118,7 +118,7 @@ static void qemu_laio_completion_cb(void *opaque)
         struct io_event events[MAX_EVENTS];
         uint64_t val;
         ssize_t ret;
-        struct timespec ts = { 0 };
+        struct timespec ts = { 0, 0 };
         int nevents, i;
 
         do {
-- 
1.7.2.2

[Qemu-devel] [PATCH 05/14] Remove unused argument for check_for_block_signature()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 block/raw.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/raw.c b/block/raw.c
index 61e6748..fc057d0 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -12,7 +12,7 @@ static int raw_open(BlockDriverState *bs, int flags)
 /* check for the user attempting to write something that looks like a
    block format header to the beginning of the image and fail out.
 */
-static int check_for_block_signature(BlockDriverState *bs, const uint8_t *buf)
+static int check_for_block_signature(const uint8_t *buf)
 {
     static const uint8_t signatures[][4] = {
         { 'Q', 'F', 'I', 0xfb }, /* qcow/qcow2 */
@@ -42,7 +42,7 @@ static int check_write_unsafe(BlockDriverState *bs, int64_t sector_num,
     }
 
     if (sector_num == 0 && nb_sectors > 0) {
-        return check_for_block_signature(bs, buf);
+        return check_for_block_signature(buf);
     }
 
     return 0;
-- 
1.7.2.2

[Qemu-devel] Re: [PATCH 05/14] Remove unused argument for check_for_block_signature()

[Anthony Liguori]

On 08/30/2010 10:59 AM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>
> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
> ---
>   block/raw.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block/raw.c b/block/raw.c
> index 61e6748..fc057d0 100644
> --- a/block/raw.c
> +++ b/block/raw.c
> @@ -12,7 +12,7 @@ static int raw_open(BlockDriverState *bs, int flags)
>   /* check for the user attempting to write something that looks like a
>      block format header to the beginning of the image and fail out.
>   */
> -static int check_for_block_signature(BlockDriverState *bs, const uint8_t *buf)
> +static int check_for_block_signature(const uint8_t *buf)
>    

This is why this type of warning sucks.  Passing BlockDriverState is a 
matter of readability because these are roughly methods.  Just because 
'this' isn't used right now, doesn't mean that it should not be a method.

Regards,

Anthony Liguori

>   {
>       static const uint8_t signatures[][4] = {
>           { 'Q', 'F', 'I', 0xfb }, /* qcow/qcow2 */
> @@ -42,7 +42,7 @@ static int check_write_unsafe(BlockDriverState *bs, int64_t sector_num,
>       }
>
>       if (sector_num == 0&&  nb_sectors>  0) {
> -        return check_for_block_signature(bs, buf);
> +        return check_for_block_signature(buf);
>       }
>
>       return 0;
>    

[Qemu-devel] Re: [PATCH 05/14] Remove unused argument for check_for_block_signature()

[Paolo Bonzini]

On 08/30/2010 06:16 PM, Anthony Liguori wrote:
> This is why this type of warning sucks.  Passing BlockDriverState is a
> matter of readability because these are roughly methods.  Just because
> 'this' isn't used right now, doesn't mean that it should not be a method.

On the contrary, to me this is acceptable (or even "a good thing") 
because a patch introducing the first use of a so-far-unused argument 
deserves a more careful review.  In fact, if we were using C++, 
check_for_block_signature should have been static.

The cases where the "this" argument is unused in a method should stand 
out as possible bugs, as is the case with the parse errors in the JSON 
parser (which _is_ a bug, as the caller cannot intercept error messages 
right now).  check_for_block_signature is not one of these.

Paolo

[Qemu-devel] Re: [PATCH 05/14] Remove unused argument for check_for_block_signature()

[Jes Sorensen]

On 08/30/10 18:40, Paolo Bonzini wrote:
> On 08/30/2010 06:16 PM, Anthony Liguori wrote:
>> This is why this type of warning sucks.  Passing BlockDriverState is a
>> matter of readability because these are roughly methods.  Just because
>> 'this' isn't used right now, doesn't mean that it should not be a method.
> 
> On the contrary, to me this is acceptable (or even "a good thing")
> because a patch introducing the first use of a so-far-unused argument
> deserves a more careful review.  In fact, if we were using C++,
> check_for_block_signature should have been static.
> 
> The cases where the "this" argument is unused in a method should stand
> out as possible bugs, as is the case with the parse errors in the JSON
> parser (which _is_ a bug, as the caller cannot intercept error messages
> right now).  check_for_block_signature is not one of these.

I totally agree on this. The problem with having such arguments passed
in is that you never know if they were used in the past and it was
forgotten when the code using them was removed, or if it's new code, in
which case they do deserve the extra scrutiny.

Cheers,
Jes

[Qemu-devel] Re: [PATCH 05/14] Remove unused argument for check_for_block_signature()

[Anthony Liguori]

On 08/30/2010 01:19 PM, Jes Sorensen wrote:
>
> I totally agree on this. The problem with having such arguments passed
> in is that you never know if they were used in the past and it was
> forgotten when the code using them was removed, or if it's new code, in
> which case they do deserve the extra scrutiny.
>    

Or, we exercise common sense instead of blinding removing arguments just 
because a certain uncommon warning mode of GCC complains.

Regards,

Anthony Liguori

> Cheers,
> Jes
>    

Re: [Qemu-devel] Re: [PATCH 05/14] Remove unused argument for check_for_block_signature()

[Richard Henderson]

On 08/30/2010 11:27 AM, Anthony Liguori wrote:
> On 08/30/2010 01:19 PM, Jes Sorensen wrote:
>> 
>> I totally agree on this. The problem with having such arguments
>> passed in is that you never know if they were used in the past and
>> it was forgotten when the code using them was removed, or if it's
>> new code, in which case they do deserve the extra scrutiny.
>> 
> 
> Or, we exercise common sense instead of blinding removing arguments
> just because a certain uncommon warning mode of GCC complains.

If you make a reasoned decision to keep the argument,
then annotate it with

#define UNUSED  __attribute__((unused))

and the warning will go away.

As to whether the argument should be retained in these
specific cases, I am agnostic.


r~

[Qemu-devel] Re: [PATCH 05/14] Remove unused argument for check_for_block_signature()

[Jes Sorensen]

On 08/30/10 20:27, Anthony Liguori wrote:
> On 08/30/2010 01:19 PM, Jes Sorensen wrote:
>>
>> I totally agree on this. The problem with having such arguments passed
>> in is that you never know if they were used in the past and it was
>> forgotten when the code using them was removed, or if it's new code, in
>> which case they do deserve the extra scrutiny.
>>    
> 
> Or, we exercise common sense instead of blinding removing arguments just
> because a certain uncommon warning mode of GCC complains.

Before making the change, I did indeed look at the code for a while,
considering whether it was reasonable to leave the unused variable in
place. However I don't see anything in there that makes it likely that
the block state parameter is going to be used in that function in the
near future, if at all.

If you feel so strongly about it, then lets apply the __unused attribute
as Richard suggested so it's clear that that argument was added on purpose.

Jes

[Qemu-devel] [PATCH 06/14] Remove unused argument for encrypt_sectors()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 block/qcow.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 816103d..c4d8cb3 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -214,9 +214,8 @@ static int qcow_set_key(BlockDriverState *bs, const char *key)
 /* The crypt function is compatible with the linux cryptoloop
    algorithm for < 4 GB images. NOTE: out_buf == in_buf is
    supported */
-static void encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
-                            uint8_t *out_buf, const uint8_t *in_buf,
-                            int nb_sectors, int enc,
+static void encrypt_sectors(int64_t sector_num, uint8_t *out_buf,
+                            const uint8_t *in_buf, int nb_sectors, int enc,
                             const AES_KEY *key)
 {
     union {
@@ -351,7 +350,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
                     memset(s->cluster_data + 512, 0x00, 512);
                     for(i = 0; i < s->cluster_sectors; i++) {
                         if (i < n_start || i >= n_end) {
-                            encrypt_sectors(s, start_sect + i,
+                            encrypt_sectors(start_sect + i,
                                             s->cluster_data,
                                             s->cluster_data + 512, 1, 1,
                                             &s->aes_encrypt_key);
@@ -474,7 +473,7 @@ static int qcow_read(BlockDriverState *bs, int64_t sector_num,
             if (ret != n * 512)
                 return -1;
             if (s->crypt_method) {
-                encrypt_sectors(s, sector_num, buf, buf, n, 0,
+                encrypt_sectors(sector_num, buf, buf, n, 0,
                                 &s->aes_decrypt_key);
             }
         }
@@ -558,7 +557,7 @@ static void qcow_aio_read_cb(void *opaque, int ret)
         /* nothing to do */
     } else {
         if (s->crypt_method) {
-            encrypt_sectors(s, acb->sector_num, acb->buf, acb->buf,
+            encrypt_sectors(acb->sector_num, acb->buf, acb->buf,
                             acb->n, 0,
                             &s->aes_decrypt_key);
         }
@@ -687,7 +686,7 @@ static void qcow_aio_write_cb(void *opaque, int ret)
                 goto done;
             }
         }
-        encrypt_sectors(s, acb->sector_num, acb->cluster_data, acb->buf,
+        encrypt_sectors(acb->sector_num, acb->cluster_data, acb->buf,
                         acb->n, 1, &s->aes_encrypt_key);
         src_buf = acb->cluster_data;
     } else {
-- 
1.7.2.2

[Qemu-devel] [PATCH 07/14] Remove unused argument for get_whole_cluster()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 block/vmdk.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 2d4ba42..54ad66f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -437,7 +437,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
                                    uint64_t offset, int allocate);
 
 static int get_whole_cluster(BlockDriverState *bs, uint64_t cluster_offset,
-                             uint64_t offset, int allocate)
+                             uint64_t offset)
 {
     BDRVVmdkState *s = bs->opaque;
     uint8_t  whole_grain[s->cluster_sectors*512];        // 128 sectors * 512 bytes each = grain size 64KB
@@ -552,7 +552,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
          * This problem may occur because of insufficient space on host disk
          * or inappropriate VM shutdown.
          */
-        if (get_whole_cluster(bs, cluster_offset, offset, allocate) == -1)
+        if (get_whole_cluster(bs, cluster_offset, offset) == -1)
             return 0;
 
         if (m_data) {
-- 
1.7.2.2

[Qemu-devel] [PATCH 08/14] Remove unused argument for qcow2_encrypt_sectors()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 block/qcow2-cluster.c |   17 ++++++++---------
 block/qcow2.c         |   10 +++++-----
 block/qcow2.h         |    7 +++----
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 166922f..14aaf7a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -319,9 +319,8 @@ static int count_contiguous_free_clusters(uint64_t nb_clusters, uint64_t *l2_tab
 /* The crypt function is compatible with the linux cryptoloop
    algorithm for < 4 GB images. NOTE: out_buf == in_buf is
    supported */
-void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
-                           uint8_t *out_buf, const uint8_t *in_buf,
-                           int nb_sectors, int enc,
+void qcow2_encrypt_sectors(int64_t sector_num, uint8_t *out_buf,
+                           const uint8_t *in_buf, int nb_sectors, int enc,
                            const AES_KEY *key)
 {
     union {
@@ -382,8 +381,8 @@ static int qcow_read(BlockDriverState *bs, int64_t sector_num,
             if (ret != n * 512)
                 return -1;
             if (s->crypt_method) {
-                qcow2_encrypt_sectors(s, sector_num, buf, buf, n, 0,
-                                &s->aes_decrypt_key);
+                qcow2_encrypt_sectors(sector_num, buf, buf, n, 0,
+                                      &s->aes_decrypt_key);
             }
         }
         nb_sectors -= n;
@@ -407,10 +406,10 @@ static int copy_sectors(BlockDriverState *bs, uint64_t start_sect,
     if (ret < 0)
         return ret;
     if (s->crypt_method) {
-        qcow2_encrypt_sectors(s, start_sect + n_start,
-                        s->cluster_data,
-                        s->cluster_data, n, 1,
-                        &s->aes_encrypt_key);
+        qcow2_encrypt_sectors(start_sect + n_start,
+                              s->cluster_data,
+                              s->cluster_data, n, 1,
+                              &s->aes_encrypt_key);
     }
     BLKDBG_EVENT(bs->file, BLKDBG_COW_WRITE);
     ret = bdrv_write_sync(bs->file, (cluster_offset >> 9) + n_start,
diff --git a/block/qcow2.c b/block/qcow2.c
index a53014d..e7ba7b4 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -397,9 +397,9 @@ static void qcow_aio_read_cb(void *opaque, int ret)
         /* nothing to do */
     } else {
         if (s->crypt_method) {
-            qcow2_encrypt_sectors(s, acb->sector_num, acb->buf, acb->buf,
-                            acb->cur_nr_sectors, 0,
-                            &s->aes_decrypt_key);
+            qcow2_encrypt_sectors(acb->sector_num, acb->buf, acb->buf,
+                                  acb->cur_nr_sectors, 0,
+                                  &s->aes_decrypt_key);
         }
     }
 
@@ -609,8 +609,8 @@ static void qcow_aio_write_cb(void *opaque, int ret)
             acb->cluster_data = qemu_mallocz(QCOW_MAX_CRYPT_CLUSTERS *
                                              s->cluster_size);
         }
-        qcow2_encrypt_sectors(s, acb->sector_num, acb->cluster_data, acb->buf,
-                        acb->cur_nr_sectors, 1, &s->aes_encrypt_key);
+        qcow2_encrypt_sectors(acb->sector_num, acb->cluster_data, acb->buf,
+                              acb->cur_nr_sectors, 1, &s->aes_encrypt_key);
         src_buf = acb->cluster_data;
     } else {
         src_buf = acb->buf;
diff --git a/block/qcow2.h b/block/qcow2.h
index 3ff162e..477b0f9 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -191,10 +191,9 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res);
 int qcow2_grow_l1_table(BlockDriverState *bs, int min_size);
 void qcow2_l2_cache_reset(BlockDriverState *bs);
 int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
-void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
-                     uint8_t *out_buf, const uint8_t *in_buf,
-                     int nb_sectors, int enc,
-                     const AES_KEY *key);
+void qcow2_encrypt_sectors(int64_t sector_num, uint8_t *out_buf,
+                           const uint8_t *in_buf, int nb_sectors, int enc,
+                           const AES_KEY *key);
 
 int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
     int *num, uint64_t *cluster_offset);
-- 
1.7.2.2

[Qemu-devel] [PATCH 09/14] Remove unused arguments for add_aio_request() and free_aio_req()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 block/sheepdog.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 81aa564..2ef8655 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -394,7 +394,7 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
     return aio_req;
 }
 
-static inline int free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
+static inline int free_aio_req(AIOReq *aio_req)
 {
     SheepdogAIOCB *acb = aio_req->aiocb;
     QLIST_REMOVE(aio_req, outstanding_aio_siblings);
@@ -755,7 +755,7 @@ out:
 }
 
 static int add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
-                           struct iovec *iov, int niov, int create,
+                           struct iovec *iov, int create,
                            enum AIOCBState aiocb_type);
 
 /*
@@ -779,10 +779,10 @@ static void send_pending_req(BDRVSheepdogState *s, uint64_t oid, uint32_t id)
 
         acb = aio_req->aiocb;
         ret = add_aio_request(s, aio_req, acb->qiov->iov,
-                              acb->qiov->niov, 0, acb->aiocb_type);
+                              0, acb->aiocb_type);
         if (ret < 0) {
             error_report("add_aio_request is failed\n");
-            free_aio_req(s, aio_req);
+            free_aio_req(aio_req);
             if (QLIST_EMPTY(&acb->aioreq_head)) {
                 sd_finish_aiocb(acb);
             }
@@ -872,7 +872,7 @@ static void aio_read_response(void *opaque)
         error_report("%s\n", sd_strerror(rsp.result));
     }
 
-    rest = free_aio_req(s, aio_req);
+    rest = free_aio_req(aio_req);
     if (!rest) {
         /*
          * We've finished all requests which belong to the AIOCB, so
@@ -1064,7 +1064,7 @@ out:
 }
 
 static int add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
-                           struct iovec *iov, int niov, int create,
+                           struct iovec *iov, int create,
                            enum AIOCBState aiocb_type)
 {
     int nr_copies = s->inode.nr_copies;
@@ -1460,9 +1460,9 @@ static void sd_write_done(SheepdogAIOCB *acb)
         iov.iov_len = sizeof(s->inode);
         aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
                                 data_len, offset, 0, 0, offset);
-        ret = add_aio_request(s, aio_req, &iov, 1, 0, AIOCB_WRITE_UDATA);
+        ret = add_aio_request(s, aio_req, &iov, 0, AIOCB_WRITE_UDATA);
         if (ret) {
-            free_aio_req(s, aio_req);
+            free_aio_req(aio_req);
             acb->ret = -EIO;
             goto out;
         }
@@ -1613,11 +1613,11 @@ static void sd_readv_writev_bh_cb(void *p)
             }
         }
 
-        ret = add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
+        ret = add_aio_request(s, aio_req, acb->qiov->iov,
                               create, acb->aiocb_type);
         if (ret < 0) {
             error_report("add_aio_request is failed\n");
-            free_aio_req(s, aio_req);
+            free_aio_req(aio_req);
             acb->ret = -EIO;
             goto out;
         }
-- 
1.7.2.2

[Qemu-devel] [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This keeps the compiler happy when building with -Wextra while
effectively generating the same code.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 qjson.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/qjson.c b/qjson.c
index e4ee433..f259547 100644
--- a/qjson.c
+++ b/qjson.c
@@ -36,8 +36,9 @@ static void parse_json(JSONMessageParser *parser, QList *tokens)
 
 QObject *qobject_from_jsonv(const char *string, va_list *ap)
 {
-    JSONParsingState state = {};
+    JSONParsingState state;
 
+    memset(&state, 0, sizeof(state));
     state.ap = ap;
 
     json_message_parser_init(&state.parser, parse_json);
-- 
1.7.2.2

[Qemu-devel] [PATCH 11/14] Remove unused function arguments

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 json-parser.c |   39 +++++++++++++++++++--------------------
 1 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/json-parser.c b/json-parser.c
index 70b9b6f..68ff9c1 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -91,7 +91,7 @@ static int token_is_escape(QObject *obj, const char *value)
 /**
  * Error handler
  */
-static void parse_error(JSONParserContext *ctxt, QObject *token, const char *msg, ...)
+static void parse_error(const char *msg, ...)
 {
     va_list ap;
     va_start(ap, msg);
@@ -165,7 +165,7 @@ static int hex2decimal(char ch)
  *      \t
  *      \u four-hex-digits 
  */
-static QString *qstring_from_escaped_str(JSONParserContext *ctxt, QObject *token)
+static QString *qstring_from_escaped_str(QObject *token)
 {
     const char *ptr = token_get_value(token);
     QString *str;
@@ -232,8 +232,7 @@ static QString *qstring_from_escaped_str(JSONParserContext *ctxt, QObject *token
                     if (qemu_isxdigit(*ptr)) {
                         unicode_char |= hex2decimal(*ptr) << ((3 - i) * 4);
                     } else {
-                        parse_error(ctxt, token,
-                                    "invalid hex escape sequence in string");
+                        parse_error("invalid hex escape sequence in string");
                         goto out;
                     }
                     ptr++;
@@ -243,7 +242,7 @@ static QString *qstring_from_escaped_str(JSONParserContext *ctxt, QObject *token
                 qstring_append(str, utf8_char);
             }   break;
             default:
-                parse_error(ctxt, token, "invalid escape sequence in string");
+                parse_error("invalid escape sequence in string");
                 goto out;
             }
         } else {
@@ -274,19 +273,19 @@ static int parse_pair(JSONParserContext *ctxt, QDict *dict, QList **tokens, va_l
     peek = qlist_peek(working);
     key = parse_value(ctxt, &working, ap);
     if (!key || qobject_type(key) != QTYPE_QSTRING) {
-        parse_error(ctxt, peek, "key is not a string in object");
+        parse_error("key is not a string in object");
         goto out;
     }
 
     token = qlist_pop(working);
     if (!token_is_operator(token, ':')) {
-        parse_error(ctxt, token, "missing : in object pair");
+        parse_error("missing : in object pair");
         goto out;
     }
 
     value = parse_value(ctxt, &working, ap);
     if (value == NULL) {
-        parse_error(ctxt, token, "Missing value in dict");
+        parse_error("Missing value in dict");
         goto out;
     }
 
@@ -331,7 +330,7 @@ static QObject *parse_object(JSONParserContext *ctxt, QList **tokens, va_list *a
         token = qlist_pop(working);
         while (!token_is_operator(token, '}')) {
             if (!token_is_operator(token, ',')) {
-                parse_error(ctxt, token, "expected separator in dict");
+                parse_error("expected separator in dict");
                 goto out;
             }
             qobject_decref(token);
@@ -384,7 +383,7 @@ static QObject *parse_array(JSONParserContext *ctxt, QList **tokens, va_list *ap
 
         obj = parse_value(ctxt, &working, ap);
         if (obj == NULL) {
-            parse_error(ctxt, token, "expecting value");
+            parse_error("expecting value");
             goto out;
         }
 
@@ -393,7 +392,7 @@ static QObject *parse_array(JSONParserContext *ctxt, QList **tokens, va_list *ap
         token = qlist_pop(working);
         while (!token_is_operator(token, ']')) {
             if (!token_is_operator(token, ',')) {
-                parse_error(ctxt, token, "expected separator in list");
+                parse_error("expected separator in list");
                 goto out;
             }
 
@@ -402,7 +401,7 @@ static QObject *parse_array(JSONParserContext *ctxt, QList **tokens, va_list *ap
 
             obj = parse_value(ctxt, &working, ap);
             if (obj == NULL) {
-                parse_error(ctxt, token, "expecting value");
+                parse_error("expecting value");
                 goto out;
             }
 
@@ -431,7 +430,7 @@ out:
     return NULL;
 }
 
-static QObject *parse_keyword(JSONParserContext *ctxt, QList **tokens)
+static QObject *parse_keyword(QList **tokens)
 {
     QObject *token, *ret;
     QList *working = qlist_copy(*tokens);
@@ -447,7 +446,7 @@ static QObject *parse_keyword(JSONParserContext *ctxt, QList **tokens)
     } else if (token_is_keyword(token, "false")) {
         ret = QOBJECT(qbool_from_int(false));
     } else {
-        parse_error(ctxt, token, "invalid keyword `%s'", token_get_value(token));
+        parse_error("invalid keyword `%s'", token_get_value(token));
         goto out;
     }
 
@@ -464,7 +463,7 @@ out:
     return NULL;
 }
 
-static QObject *parse_escape(JSONParserContext *ctxt, QList **tokens, va_list *ap)
+static QObject *parse_escape(QList **tokens, va_list *ap)
 {
     QObject *token = NULL, *obj;
     QList *working = qlist_copy(*tokens);
@@ -507,7 +506,7 @@ out:
     return NULL;
 }
 
-static QObject *parse_literal(JSONParserContext *ctxt, QList **tokens)
+static QObject *parse_literal(QList **tokens)
 {
     QObject *token, *obj;
     QList *working = qlist_copy(*tokens);
@@ -515,7 +514,7 @@ static QObject *parse_literal(JSONParserContext *ctxt, QList **tokens)
     token = qlist_pop(working);
     switch (token_get_type(token)) {
     case JSON_STRING:
-        obj = QOBJECT(qstring_from_escaped_str(ctxt, token));
+        obj = QOBJECT(qstring_from_escaped_str(token));
         break;
     case JSON_INTEGER:
         obj = QOBJECT(qint_from_int(strtoll(token_get_value(token), NULL, 10)));
@@ -550,13 +549,13 @@ static QObject *parse_value(JSONParserContext *ctxt, QList **tokens, va_list *ap
         obj = parse_array(ctxt, tokens, ap);
     }
     if (obj == NULL) {
-        obj = parse_escape(ctxt, tokens, ap);
+        obj = parse_escape(tokens, ap);
     }
     if (obj == NULL) {
-        obj = parse_keyword(ctxt, tokens);
+        obj = parse_keyword(tokens);
     } 
     if (obj == NULL) {
-        obj = parse_literal(ctxt, tokens);
+        obj = parse_literal(tokens);
     }
 
     return obj;
-- 
1.7.2.2

[Qemu-devel] [PATCH 12/14] size_t is unsigned, change to ssize_t to handle errors from tight_compress_data()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 ui/vnc-enc-tight.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index c4c9c3b..c942bb7 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -905,7 +905,7 @@ static void tight_pack24(VncState *vs, uint8_t *buf, size_t count, size_t *ret)
 static int send_full_color_rect(VncState *vs, int x, int y, int w, int h)
 {
     int stream = 0;
-    size_t bytes;
+    ssize_t bytes;
 
 #ifdef CONFIG_VNC_PNG
     if (tight_can_send_png_rect(vs, w, h)) {
@@ -949,7 +949,7 @@ static int send_solid_rect(VncState *vs)
 static int send_mono_rect(VncState *vs, int x, int y,
                           int w, int h, uint32_t bg, uint32_t fg)
 {
-    size_t bytes;
+    ssize_t bytes;
     int stream = 1;
     int level = tight_conf[vs->tight.compression].mono_zlib_level;
 
@@ -1029,7 +1029,7 @@ static bool send_gradient_rect(VncState *vs, int x, int y, int w, int h)
 {
     int stream = 3;
     int level = tight_conf[vs->tight.compression].gradient_zlib_level;
-    size_t bytes;
+    ssize_t bytes;
 
     if (vs->clientds.pf.bytes_per_pixel == 1)
         return send_full_color_rect(vs, x, y, w, h);
@@ -1066,7 +1066,7 @@ static int send_palette_rect(VncState *vs, int x, int y,
     int stream = 2;
     int level = tight_conf[vs->tight.compression].idx_zlib_level;
     int colors;
-    size_t bytes;
+    ssize_t bytes;
 
 #ifdef CONFIG_VNC_PNG
     if (tight_can_send_png_rect(vs, w, h)) {
-- 
1.7.2.2

[Qemu-devel] [PATCH 13/14] Change DPRINTF() to do{}while(0) to avoid compiler warning

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 slirp/bootp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/slirp/bootp.c b/slirp/bootp.c
index 3e4e881..41460ff 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -33,7 +33,7 @@ static const uint8_t rfc1533_cookie[] = { RFC1533_COOKIE };
 #define DPRINTF(fmt, ...) \
 do if (slirp_debug & DBG_CALL) { fprintf(dfd, fmt, ##  __VA_ARGS__); fflush(dfd); } while (0)
 #else
-#define DPRINTF(fmt, ...)
+#define DPRINTF(fmt, ...) do{}while(0)
 #endif
 
 static BOOTPClient *get_new_addr(Slirp *slirp, struct in_addr *paddr,
-- 
1.7.2.2

[Qemu-devel] [PATCH 14/14] load_multiboot(): get_image_size() returns int

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Do not store return of get_image_size() in a uint32_t as it makes it
impossible to detect error returns from get_image_size.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 hw/multiboot.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/multiboot.c b/hw/multiboot.c
index dc980e6..f9097a2 100644
--- a/hw/multiboot.c
+++ b/hw/multiboot.c
@@ -252,7 +252,7 @@ int load_multiboot(void *fw_cfg,
 
         do {
             char *next_space;
-            uint32_t mb_mod_length;
+            int mb_mod_length;
             uint32_t offs = mbs.mb_buf_size;
 
             next_initrd = strchr(initrd_filename, ',');
-- 
1.7.2.2

[Qemu-devel] Re: [PATCH 00/14] gcc extra warning fixes v2

[Kevin Wolf]

Am 30.08.2010 17:59, schrieb Jes.Sorensen@redhat.com:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Hi,
> 
> I started building QEMU with some more aggressive error flags to see
> what dropped out. I started fixing up some of the cases, removing
> unused arguments to functions, comparisons of unsigned types against
> negative values etc. and a few other minor changes to avoid compiler
> warnings.
> 
> v2 fixes the vnc change as pointed out by Anthony.
> 
> Set of 14 patches following.
> 
> Cheers,
> Jes
> 
> Jes Sorensen (14):
>   Remove unused argument for nbd_client()
>   Respect return value from nbd_client()
>   Fix repeated typo: was "end if list" instead of "end of list"
>   Zero initialize timespec struct explicitly
>   Remove unused argument for check_for_block_signature()
>   Remove unused argument for encrypt_sectors()
>   Remove unused argument for get_whole_cluster()
>   Remove unused argument for qcow2_encrypt_sectors()
>   Remove unused arguments for add_aio_request() and free_aio_req()
>   Zero json struct with memset() instea of = {} to keep compiler happy.
>   Remove unused function arguments
>   size_t is unsigned, change to ssize_t to handle errors from
>     tight_compress_data()
>   Change DPRINTF() to do{}while(0) to avoid compiler warning
>   load_multiboot(): get_image_size() returns int

For the block related ones:

Acked-by: Kevin Wolf <kwolf@redhat.com>

[Qemu-devel] [PATCH 00/14] gcc extra warning fixes

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi,

I started building QEMU with some more aggressive error flags to see
what dropped out. I started fixing up some of the cases, removing
unused arguments to functions, comparisons of unsigned types against
negative values etc. and a few other minor changes to avoid compiler
warnings.

Set of 14 patches following.

Cheers,
Jes

Jes Sorensen (14):
  Remove unused argument for nbd_client()
  Respect return value from nbd_client()
  Fix repeated typo: was "end if list" instead of "end of list"
  Zero initialize timespec struct explicitly
  Remove unused argument for check_for_block_signature()
  Remove unused argument for encrypt_sectors()
  Remove unused argument for get_whole_cluster()
  Remove unused argument for qcow2_encrypt_sectors()
  Remove unused arguments for add_aio_request() and free_aio_req()
  Zero json struct with memset() instea of = {} to keep compiler happy.
  Remove unused function arguments
  size_t is unsigned, so (foo >= 0) is always true
  Change DPRINTF() to do{}while(0) to avoid compiler warning
  load_multiboot(): get_image_size() returns int

 block/qcow.c          |   13 ++++++-------
 block/qcow2-cluster.c |   17 ++++++++---------
 block/qcow2.c         |   10 +++++-----
 block/qcow2.h         |    7 +++----
 block/raw.c           |    4 ++--
 block/sheepdog.c      |   20 ++++++++++----------
 block/vmdk.c          |    4 ++--
 hw/multiboot.c        |    2 +-
 json-parser.c         |   39 +++++++++++++++++++--------------------
 linux-aio.c           |    2 +-
 nbd.c                 |    4 ++--
 nbd.h                 |    2 +-
 qemu-config.c         |   12 ++++++------
 qemu-nbd.c            |    5 ++++-
 qjson.c               |    3 ++-
 slirp/bootp.c         |    2 +-
 ui/vnc-enc-tight.c    |    8 ++++----
 17 files changed, 77 insertions(+), 77 deletions(-)

-- 
1.7.2.2

[Qemu-devel] [PATCH 07/14] Remove unused argument for get_whole_cluster()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 block/vmdk.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 2d4ba42..54ad66f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -437,7 +437,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
                                    uint64_t offset, int allocate);
 
 static int get_whole_cluster(BlockDriverState *bs, uint64_t cluster_offset,
-                             uint64_t offset, int allocate)
+                             uint64_t offset)
 {
     BDRVVmdkState *s = bs->opaque;
     uint8_t  whole_grain[s->cluster_sectors*512];        // 128 sectors * 512 bytes each = grain size 64KB
@@ -552,7 +552,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
          * This problem may occur because of insufficient space on host disk
          * or inappropriate VM shutdown.
          */
-        if (get_whole_cluster(bs, cluster_offset, offset, allocate) == -1)
+        if (get_whole_cluster(bs, cluster_offset, offset) == -1)
             return 0;
 
         if (m_data) {
-- 
1.7.2.2