Re: [libvirt] [PATCH v2 3/3] virt-aa-helper: drop pointer checks in get_files
On 11/19/19 4:31 PM, Jamie Strandboge wrote:
On Thu, 14 Nov 2019, Christian Ehrhardt wrote:
> It was mentioned that the pointers in loops like:
> for (i = 0; i < ctl->def->nserials; i++)
> if (ctl->def->serials[i] ...
> will always be !NULL or we would have a much more serious problem.
>
> Simplify the if chains in get_files by dropping these checks.
I don't really see why a NULL check is a problem so this feels a bit
like busy work and it seems short-circuiting and not doing is exactly
what you would want to do in the event of a 'much more serious problem'.
Is this really required? +0 to apply on principle (I've not reviewed the
changes closely).
If for example def->nserials and def->serials[i] disagree, then there is
a serious bug somewhere. The internal API contract is that it should
never happen, so code shouldn't check for the condition. I'm pretty sure
if the XML parser is creating that situation, the qemu driver would
crash in a dozen different places.
So the patch isn't strictly required, but it is an improvement IMO: it
reduces code, improves readability, and helps simplify review for other
libvirt devs who may be confused by this uncommon idiom (I was). But I
will leave it up to you guys to decide whether to push or not
Thanks,
Cole