[libvirt] [PATCH] remove useless code

[Daniel P. Berrange]

On Thu, Feb 05, 2009 at 01:55:39PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange at redhat.com> wrote:
> ...
> > These two are not safe to remove. Look at the whole code block in
> > question:
> >
> >     if (diskVal->list == NULL)
> >         VIR_FREE(diskVal);
> >     else if (virConfSetValue(conf, "disk", diskVal) < 0) {
> >         diskVal = NULL;
> >         goto no_memory;
> >     }
> >     diskVal = NULL;
> >
> > In the case where virConfSetValue returned >= 0, we need to
> > still set diskVal = NULL.
> 
> Oops.  Thanks.
> 
> However, the original code is ugly enough that I have rewritten
> it to make it clear that regardless of the virConfSetValue outcome,
> we don't free that variable:
> 
>     if (diskVal->list != NULL) {
>         bool err = (virConfSetValue(conf, "disk", diskVal) < 0);
>         diskVal = NULL;
>         if (err)
>             goto no_memory;
>     }
>     VIR_FREE(diskVal);

I prefer if it had the return value check separate from the assignment,
eg

         int ret = virConfSetValue(conf, "disk", diskVal);
         diskVal = NULL;
         if (ret < 0)
             goto no_memory;

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|