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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]