On 07/11/2012 07:35 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> This converts the following public API datatypes to use the
> virObject infrastructure:
>
> @@ -426,6 +316,9 @@ virInterfacePtr
> virGetInterface(virConnectPtr conn, const char *name, const char *mac) {
Resuming where I left off...
> @@ -558,7 +389,9 @@ virStoragePoolPtr
> virGetStoragePool(virConnectPtr conn, const char *name,
> const unsigned char *uuid) {
> virStoragePoolPtr ret = NULL;
> - char uuidstr[VIR_UUID_STRING_BUFLEN];
> +
> + if (virDataTypesInitialize() < 0)
> + return NULL;
Another instance where we should either remove the uuidstr
independently, or start logging it to make the logs more useful.
> virStreamPtr virGetStream(virConnectPtr conn) {
As long as you are touching this area of code, is it worth giving this
function consistent formatting?
virStreamPtr
virGetStream(virConnectPtr conn)
{
> +++ b/src/datatypes.h
> +# define VIR_IS_NETWORK(obj) \
> + (virObjectIsClass((virObjectPtr)obj, virNetworkClass))
Technically under-parenthesized, should be:
(virObjectIsClass((virObjectPtr)(obj), virNetworkClass))
in case 'obj' expands to a non-trivial expression. (Then again, our
likelihood of using this macro on a non-trivial expression is
practically zero). Same goes for all the other VIR_IS_* macros wrapping
virObjectIsClass.
> +++ b/src/libvirt.c
> @@ -1430,10 +1430,9 @@ virConnectClose(virConnectPtr conn)
> goto error;
> }
>
> - ret = virUnrefConnect(conn);
> - if (ret < 0)
> - goto error;
> - return ret;
> + if (!virObjectUnref(conn))
> + return 0;
> + return 1;
Here is where you are changing the return value; you need to update the
documentation to match.
> +++ b/src/libvirt_private.syms
> +virSecretClass;
> +virStreamClass;
> +virStorageVolClass;
> +virStoragePoolClass;
Not quite sorted.
Mostly mechanical, and overall a nice patch, but I did point out one or
two issues worth fixing for v2 (at which point, I will just review the
interdiff).
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list