Hi
> On Thu, Nov 10, 2011 at 09:33:42PM +0100, Marc-André Lureau wrote:
> > + case PROP_PATH:
> > + if (priv->path)
> > + g_free(priv->path);
>
> You can safely call g_free on a NULL pointer, this makes the code a
> bit
> simpler, there are several occurrences of this in the 2 patches.
Correct, I forgot. I will fix that.
> > +static GVirDomainInterfaceStats *
> > +gvir_domain_interface_stats_copy(GVirDomainInterfaceStats *stats)
> > +{
> > + return g_slice_dup(GVirDomainInterfaceStats, stats);
> > +}
>
> Do we really need to use GSlice here? I consider GSlice as something
> to use
> when you want to make many allocations of same-size objects, will we
> allocate many of these stats objects?
Yes, it's frequently allocated (1/second), and kept in memory too for history.
> > +typedef struct _GVirDomainInterfaceStats GVirDomainInterfaceStats;
> > +struct _GVirDomainInterfaceStats
> > +{
> > + gint64 rx_bytes;
> > + gint64 rx_packets;
> > + gint64 rx_errs;
> > + gint64 rx_drop;
> > + gint64 tx_bytes;
> > + gint64 tx_packets;
> > + gint64 tx_errs;
> > + gint64 tx_drop;
> > +};
>
> 2 questions about this:
> * should we use more explicit names (which probably means longer
> ones)?
I don't mind. I just copy&pasted libvirt here, which I like because you can directly map it to libvirt struct.
> * how do we handle ABI compatibility the day we need to add fields to
> this
> struct?
It's only a returned structure, allocated by the lib, so I guess that's fine to append fields later, right?
regards
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list