Re: [libvirt] [PATCH libvirt-glib 2/3] Add GVirDomainInterface
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