[...]
>>> 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@xxxxxxxxxx>
>
> 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
> }
>
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
