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