On Wed, 20 Nov 2019, Cole Robinson wrote: > 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 To be clear, I am intentionally not blocking on this. If, as upstream, you'd prefer this to be a certain way for reasons that outweigh my POV, by all means feel free to do that. The changes are mechanical and IMHO don't need an apparmor-focused review (though if you'd prefer I go through the full patch, I can). -- Jamie Strandboge | http://www.canonical.com
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
