[Qemu-devel] [PATCH] migration: fix warning for source_return_path_thread

[Qemu-devel] [PATCH] migration: fix warning for source_return_path_thread

[Peter Xu]

max_len is not necessary, while it brings a warning during compilation
when specify "-Wstack-usage=1000000". Replacing using sizeof().

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 0129d9f..6680d95 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1268,8 +1268,7 @@ static void *source_return_path_thread(void *opaque)
     MigrationState *ms = opaque;
     QEMUFile *rp = ms->rp_state.from_dst_file;
     uint16_t header_len, header_type;
-    const int max_len = 512;
-    uint8_t buf[max_len];
+    uint8_t buf[512];
     uint32_t tmp32, sibling_error;
     ram_addr_t start = 0; /* =0 to silence warning */
     size_t  len = 0, expected_len;
@@ -1292,7 +1291,7 @@ static void *source_return_path_thread(void *opaque)
 
         if ((rp_cmd_args[header_type].len != -1 &&
             header_len != rp_cmd_args[header_type].len) ||
-            header_len > max_len) {
+            header_len > sizeof(buf)) {
             error_report("RP: Received '%s' message (0x%04x) with"
                     "incorrect length %d expecting %zu",
                     rp_cmd_args[header_type].name, header_type, header_len,
-- 
2.4.3

Re: [Qemu-devel] [PATCH] migration: fix warning for source_return_path_thread

[Paolo Bonzini]

On 09/03/2016 07:12, Peter Xu wrote:
> max_len is not necessary, while it brings a warning during compilation
> when specify "-Wstack-usage=1000000". Replacing using sizeof().
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 0129d9f..6680d95 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1268,8 +1268,7 @@ static void *source_return_path_thread(void *opaque)
>      MigrationState *ms = opaque;
>      QEMUFile *rp = ms->rp_state.from_dst_file;
>      uint16_t header_len, header_type;
> -    const int max_len = 512;
> -    uint8_t buf[max_len];
> +    uint8_t buf[512];
>      uint32_t tmp32, sibling_error;
>      ram_addr_t start = 0; /* =0 to silence warning */
>      size_t  len = 0, expected_len;
> @@ -1292,7 +1291,7 @@ static void *source_return_path_thread(void *opaque)
>  
>          if ((rp_cmd_args[header_type].len != -1 &&
>              header_len != rp_cmd_args[header_type].len) ||
> -            header_len > max_len) {
> +            header_len > sizeof(buf)) {

sizeof works fine because buf is an array of bytes, but ARRAY_SIZE is
better because it works for any type of buffer.

Paolo

>              error_report("RP: Received '%s' message (0x%04x) with"
>                      "incorrect length %d expecting %zu",
>                      rp_cmd_args[header_type].name, header_type, header_len,
> 

Re: [Qemu-devel] [PATCH] migration: fix warning for source_return_path_thread

[Peter Xu]

On Wed, Mar 09, 2016 at 08:54:51AM +0100, Paolo Bonzini wrote:
> > @@ -1292,7 +1291,7 @@ static void *source_return_path_thread(void *opaque)
> >  
> >          if ((rp_cmd_args[header_type].len != -1 &&
> >              header_len != rp_cmd_args[header_type].len) ||
> > -            header_len > max_len) {
> > +            header_len > sizeof(buf)) {
> 
> sizeof works fine because buf is an array of bytes, but ARRAY_SIZE is
> better because it works for any type of buffer.

I carefully chose "sizeof" out of "ARRAY_SIZE" as commented by Eric
that, we'd better use sizeof() for char typed buffers.

Will this be related to how header_len is defined? That's something
I do not know... :( E.g., if it's defined as "length in bytes", then
shall we better use sizeof() in all cases?

Anyway, I take my above question as trivial since current buffer is
char typed.

Thanks!
Peter

Re: [Qemu-devel] [PATCH] migration: fix warning for source_return_path_thread

[Paolo Bonzini]

On 09/03/2016 09:26, Peter Xu wrote:
>>> > >          if ((rp_cmd_args[header_type].len != -1 &&
>>> > >              header_len != rp_cmd_args[header_type].len) ||
>>> > > -            header_len > max_len) {
>>> > > +            header_len > sizeof(buf)) {
>> > 
>> > sizeof works fine because buf is an array of bytes, but ARRAY_SIZE is
>> > better because it works for any type of buffer.
> I carefully chose "sizeof" out of "ARRAY_SIZE" as commented by Eric
> that, we'd better use sizeof() for char typed buffers.
> 
> Will this be related to how header_len is defined? That's something
> I do not know... :( E.g., if it's defined as "length in bytes", then
> shall we better use sizeof() in all cases?

Oops, I was wrong. It's related to how header_len is used.  header_len
here is clearly a size in bytes (it's passed to qemu_get_buffer), so the
patch is okay.

Paolo

> Anyway, I take my above question as trivial since current buffer is
> char typed.