Re: [Qemu-trivial] [PATCH] arch_init: Clean up the duplicate variable 'len' defining in ram_load()

[Michael Tokarev]

15.05.2015 12:00, zhanghailiang wrote:
> There are two places that define 'len' variable, It's OK for compiling,
> but makes it difficult for reading.
> 
> Remove the local one which defined in the inside 'while' loop.
> 
> Signed-off-by: zhanghailiang <address@hidden>
> ---
>  arch_init.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 23d3feb..8918f6c 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -1593,7 +1593,6 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> version_id)
>              total_ram_bytes = addr;
>              while (!ret && total_ram_bytes) {
>                  RAMBlock *block;
> -                uint8_t len;

Note the type of this variable is not the same as the "other" len,
which is of type `int'.  Note also that at least in theory, signed
type might show different behavour than unsigned, for bytes (in
this case) with high (8th) bit set.  I just briefly looked at
what the code does, it assigns result of qemu_get_byte() to
this `len' variable, and this ensures it is interpreted as
unsigned.  By removing this `len' variable and using `len' of
type `int', it is possible this will change.  A bit more review
is needed here, I think.  If time permits I'll take a look, but
I can't do it right now.

Thanks,

/mjt