[libvirt] [PATCH] remove useless code

[Daniel P. Berrange]

On Thu, Feb 05, 2009 at 05:01:55PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange at redhat.com> wrote:
> 
> > 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;
> 
> Ok. so do I.
> 
> 
> From 6f1b5bb867d58d5606986ec1c8f34f6d95007663 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at redhat.com>
> Date: Wed, 4 Feb 2009 17:44:24 +0100
> Subject: [PATCH] remove useless code
> 
> * src/bridge.c (brAddTap): Remove redundant errno=ENOMEM assignment
> after failed strdup.
> * src/cgroup.c (virCgroupFree): Remove redundant *group=NULL assignment
> after VIR_FREE(*group).
> * src/lxc_driver.c (lxcShutdown): Likewise.
> * src/xen_unified.c (xenUnifiedClose): Likewise.
> * src/xm_internal.c (xenXMDomainConfigFormat): Rewrite disk-
> and net-config-setting code to be clearer.
> ---
>  src/bridge.c      |    6 ++----
>  src/cgroup.c      |    1 -
>  src/lxc_driver.c  |    1 -
>  src/xen_unified.c |    1 -
>  src/xm_internal.c |   21 ++++++++++-----------
>  5 files changed, 12 insertions(+), 18 deletions(-)


ACK

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 :|