On Fri, Jun 30, 2017 at 12:34:15PM +0200, Andrea Bolognani wrote:
On Fri, 2017-06-30 at 10:18 +0200, Ján Tomko wrote:> Same as virDomainDeviceInfo itself, any struct that > embeds it needs to be initialized properly before use; > however, none of the structs in question even had a > proper allocation function defined. > > Implement an allocation function for all structs > embedding a virDomainDeviceInfo and use them instead > of plain VIR_ALLOC() everywhere. NACK This is a pointless obfuscationWould you mind spending a few words to explain why you feel that's the case? Having a function where you perform both allocation and initialization of your data structure is a pretty common
As of now, nothing special is initialized in the structure. If you are going to need initialize something in the future, please mention it also in the commit message, not just the cover letter. This makes the obfuscation pointful.
pattern both outside and inside libvirt, and while it adds one layer of indirection it also improves flexibility and encapsulation, which makes it IMHO well worth it.
I am not a fan of adding code just in case we might need it in the future, which is what this patch looks like to a lazy reviewer that does not read cover letters for cleanups/refactors. Jan
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
