From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <1393347170-28502-2-git-send-email-a.rigo@virtualopensystems.com>
	(Alvise Rigo's message of "Tue, 25 Feb 2014 17:52:47 +0100")
References: <1393347170-28502-1-git-send-email-a.rigo@virtualopensystems.com>
	<1393347170-28502-2-git-send-email-a.rigo@virtualopensystems.com>
Date: Tue, 25 Feb 2014 19:52:18 +0100
Message-ID: <87k3cjrpnh.fsf@elfo.mitica>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [RFC 1/4] Fix issue affecting get_int32_le() in
	vmstate.c
Reply-To: quintela@redhat.com
To: Alvise Rigo <a.rigo@virtualopensystems.com>
Cc: Orit Wasserman <owasserm@redhat.com>, tech@virtualopensystems.com, qemu-devel@nongnu.org, Eduardo Habkost <ehabkost@redhat.com>

Alvise Rigo <a.rigo@virtualopensystems.com> wrote:
> The method is not behaving in the way it's supposed to. It should return
> the new value only if it's less than the actual one.
>
> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>

See David patch of this function.  There were a bug, we were doing the
wrong comparison.  But we expect not to chang the local value.  We just
want the the one that cames is less or equal that the current value
(think of an array size, it is a bad idea to try to read a bigger array
into a smaller one).

BTW, did you find this bug by testing or by code inspection?

thanks, Juan.

> ---
>  vmstate.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/vmstate.c b/vmstate.c
> index 284b080..038b274 100644
> --- a/vmstate.c
> +++ b/vmstate.c
> @@ -326,11 +326,11 @@ const VMStateInfo vmstate_info_int32_equal = {
>  
>  static int get_int32_le(QEMUFile *f, void *pv, size_t size)
>  {
> -    int32_t *old = pv;
> -    int32_t new;
> -    qemu_get_sbe32s(f, &new);
> +    int32_t old = *(int32_t *)pv;
> +    int32_t *new = pv;
> +    qemu_get_sbe32s(f, new);
>  
> -    if (*old <= new) {
> +    if (*new <= old) {
>          return 0;
>      }
>      return -EINVAL;