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