On Mon, 2021-03-15 at 14:28 +0100, Michal Privoznik wrote:
> On 3/15/21 12:22 PM, Andrea Bolognani wrote:
> > if (virFileReadAllQuiet(procfile, 2048, &buf) < 0)
> > - return -1;
> > + goto error;
>
> virFileReadAllQuiet() sets errno, this would just overwrite it with
> something less specific.
Good point. I've changed this to
if (virFileReadAllQuiet(procfile, 2048, &buf) < 0) {
/* This function already sets errno, so don't overwrite that
* and return immediately instead */
return -1;
}
so that some explanation for the choice is retained.
> > if (!(lines = g_strsplit(buf, "\n", 0)))
> > - return -1;
> > + goto error;
>
> I think this check can be dropped. g_strsplit() doesn't ever return NULL
> really, does it?
You're right, it doesn't. I've changed it.
> The rest looks good.
Thanks for the review and for the very good suggestions!
I'll see whether I can get some feedback from KubeVirt developers
regarding whether or not this actually improves things for them
before pushing.
--
Andrea Bolognani / Red Hat / Virtualization
