[libvirt] [PATCH v1 04/32] util: macaddr: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC

[Andrea Bolognani]

On Fri, 2018-08-03 at 09:51 +0200, Erik Skultety wrote:
> > > > +void
> > > > +virMacAddrFree(virMacAddrPtr addr)
> > > > +{
> > > > +    VIR_FREE(addr);
> > > > +}
> > > 
> > > I understand the reason behind this change, however, I don't feel like this
> > > will bring any benefits only because we said that VIR_AUTOFREE should be used
> > > with scalar types only, I'd prefer simply using VIR_AUTOFREE here, CC'ng Pavel
> > > to share his opinion.
> 
> Sigh, I just realized we already have virPCIDeviceAddressFree which does the
> very same thing (possibly more of those) which I missed during the review
> of the previous batches. Oh well, we might have to go with this in the end in
> order not to cause more confusion.

This kind of change is *very good*. Two main reasons for that:

  * Consistency. Whenever you want to free a virSomething, you
    should be able to just call virSomethingFree() without having
    to look up whether or not virSomething contains dynamically
    allocated memory.

  * Extensibility. virMacAddrFree() might not do much *right now*,
    but there's no guarantee that at some point down the line
    virMacAddr won't need to allocate some memory[1], at which
    point you'd have to introduce the function anyway as a
    prerequisite for your actual changes, in addition of course
    to tracking down all uses and convert them from VIR_AUTOFREE()
    and VIR_FREE() to VIR_AUTOPTR() and virMacAddrFree().

I'd say that's well worth adding a few extra lines.


[1] Well, virMacAddr specifically probably won't :) But that's
    definitely going to be the case for other types (and also
    see the first point again :)
-- 
Andrea Bolognani / Red Hat / Virtualization