[libvirt] [PATCH 2/3] lxc: Resolve memory leak

[John Ferlan]

[...]

>>> the comments further onward would be simply the same, you get the picture, but
>>> those are just tiny nitpicks based on personal preference I'd say (except for
>>> copying the same error messages all over again), with that addressed:
>>
>> Nope, the picture wasn't clear, but I can/should check my coffee to see
>> if it's strong enough.
> 
> Well, I was a bit too fast with my thinking, missing some obvious problems
> which I'd have spotted right away had I tried to write the code myself, so now
> I see that it couldn't have worked using my way, but for completeness, you can
> find a diff that more-or-less summarizes my original points below (also in the
> attachment for easy application), we can potentially discuss that, but at this
> point I'm more convinced by your version that by mine. Anyway, let me know
> what your thoughts are, otherwise:

Ahhh - now I see... Since I hadn't followed all the VIR_AUTOFREE stuff
that closely - I figured perhaps from the few examples that I'd seen
that each virConfGetValueString would have a uniquely named autofree
pointer rather than using a common name and using VIR_FREE "as needed"
and then reloading. Having a unique name just didn't seem that "costly".

> 
> Reviewed-by: Erik Skultety <eskultet at redhat.com>
> 
> diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
> index 6c5640536b..8c9d736d5c 100644
> --- a/src/lxc/lxc_native.c
> +++ b/src/lxc/lxc_native.c
> @@ -219,7 +219,7 @@ lxcConvertSize(const char *size, unsigned long long *value)
> 
>      /* Split the string into value and unit */
>      if (virStrToLong_ull(size, &unit, 10, value) < 0)
> -        goto error;
> +        return -1;
> 
>      if (STREQ(unit, "%")) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -228,16 +228,10 @@ lxcConvertSize(const char *size, unsigned long long *value)
>          return -1;
>      } else {
>          if (virScaleInteger(value, unit, 1, ULLONG_MAX) < 0)
> -            goto error;
> +            return -1;
>      }
> 
>      return 0;
> -
> - error:
> -    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                   _("failed to convert size: '%s'"),
> -                   size);
> -    return -1;

I actually figured this was a better model rather than the other way, so
what I did was create a "pre" patch to this one that removed the error
message from lxcSetMemTune and just "return -1;" directly.

I can just send a v2 with the lxc_native.c changes - it should take all
of about 10 seconds to look at ;-)

John

>  }
> 

[...]