[libvirt] [PATCH libvirt-glib 2/3] Add GVirDomainInterface

[Marc-André Lureau]

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