[...]
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@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
}
[...]