Re: [libvirt] [libvirt-sandbox PATCH V2] Add filter support.
On Thu, Nov 21, 2013 at 09:58:45AM +0100, Christophe Fergeau wrote:
Hey,
Here is a review for the 2 new gobjects you introduced (given the size of
the patch, I'd tend to split the introduction of these 2 new objects in a
separate patch, this would make the patch with changes to the existing code
much smaller).
Christophe
[snip]
> +{
> + GVirSandboxConfigNetworkFilterrefPrivate *priv = config->priv;
> + return priv->filter;
> +}
I don't know what the convention is in libvirt-sandbox, but
spice-gtk/libvirt-glib try to protect public API calls with
g_return_val_if_fail(GVIR_SANDBOX_IS_CONFIG_NETWORK_FILTER(config), NULL);
so that invalid objects are caught early on
I notice the whole codebase for libvirt-sandbox doesn't have these
checks. Might be worth going through and fixing them at some point. In
fact most of the things you mentioned in the review could be applied to
most of the code base. I'll see how things go, I may do a clean up
patch if others think it's a good idea.
Ian