Re: [Qemu-devel] [PATCH 2/3] rng-egd: Free old chr_name value before setting new one

[Markus Armbruster]

Eduardo Habkost <address@hidden> writes:

> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
> Cc: Anthony Liguori <address@hidden>
> Cc: Amos Kong <address@hidden>
> ---
>  backends/rng-egd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/backends/rng-egd.c b/backends/rng-egd.c
> index 25bb3b4..2962795 100644
> --- a/backends/rng-egd.c
> +++ b/backends/rng-egd.c
> @@ -169,6 +169,7 @@ static void rng_egd_set_chardev(Object *obj, const char 
> *value, Error **errp)
>      if (b->opened) {
>          error_set(errp, QERR_PERMISSION_DENIED);
>      } else {
> +        g_free(s->chr_name);
>          s->chr_name = g_strdup(value);
>      }
>  }

I see you searched for the bug elsewhere.  Appreciated.  Do you think
you got them all?

Aside: I hate

    if (bad) {
        handle error
    } else {
        continue normal work
    }

The normal flow gets nested deeper and deeper.  When the code grows,
normal work can easily end up after the conditional by mistake.  Better:

    if (bad) {
        handle error
        return
    }
    continue normal work

Anyway, not your fault.

Reviewed-by: Markus Armbruster <address@hidden>