maint: avoid nested public calls

Having one API call into another is generally not good; among
other issues, it gives confusing logs, and is not quite as
efficient.

This fixes several instances, but not all: we still have instances
in both libvirt.c and in backend hypervisors (lxc and qemu) calling
the public virTypedParamsGetString and friends, which dispatch
errors immediately.  I'm not sure if it is worth trying to clean
that up in a separate patch (such a cleanup may be easiest by
separating the public function into a wrapper around the internal,
then tweaking internal.h so that internal users directly use the
internal function).

* src/libvirt.c (virDomainGetUUIDString, virNetworkGetUUIDString)
(virStoragePoolGetUUIDString, virSecretGetUUIDString)
(virNWFilterGetUUIDString): Avoid nested public API call.
* src/util/virtypedparam.c (virTypedParamsReplaceString): Don't
dispatch errors here.
(virTypedParamsGet): No need to reset errors.
(virTypedParamsGetBoolean): Use consistent ordering.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/src/libvirt.c b/src/libvirt.c
index a0a26e584cc4bbfede7c75d53829fd53305c115a..3fb78f91ac4ad8925e27f3815a49523199fe9629 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3627,8 +3627,6 @@ error:
 int
 virDomainGetUUIDString(virDomainPtr domain, char *buf)
 {
-    unsigned char uuid[VIR_UUID_BUFLEN];
-
     VIR_DOMAIN_DEBUG(domain, "buf=%p", buf);
 
     virResetLastError();
@@ -3640,10 +3638,7 @@ virDomainGetUUIDString(virDomainPtr domain, char *buf)
     }
     virCheckNonNullArgGoto(buf, error);
 
-    if (virDomainGetUUID(domain, &uuid[0]))
-        goto error;
-
-    virUUIDFormat(uuid, buf);
+    virUUIDFormat(domain->uuid, buf);
     return 0;
 
 error:
@@ -12202,7 +12197,6 @@ error:
 int
 virNetworkGetUUIDString(virNetworkPtr network, char *buf)
 {
-    unsigned char uuid[VIR_UUID_BUFLEN];
     VIR_DEBUG("network=%p, buf=%p", network, buf);
 
     virResetLastError();
@@ -12214,10 +12208,7 @@ virNetworkGetUUIDString(virNetworkPtr network, char *buf)
     }
     virCheckNonNullArgGoto(buf, error);
 
-    if (virNetworkGetUUID(network, &uuid[0]))
-        goto error;
-
-    virUUIDFormat(uuid, buf);
+    virUUIDFormat(network->uuid, buf);
     return 0;
 
 error:
@@ -14294,7 +14285,6 @@ int
 virStoragePoolGetUUIDString(virStoragePoolPtr pool,
                             char *buf)
 {
-    unsigned char uuid[VIR_UUID_BUFLEN];
     VIR_DEBUG("pool=%p, buf=%p", pool, buf);
 
     virResetLastError();
@@ -14306,10 +14296,7 @@ virStoragePoolGetUUIDString(virStoragePoolPtr pool,
     }
     virCheckNonNullArgGoto(buf, error);
 
-    if (virStoragePoolGetUUID(pool, &uuid[0]))
-        goto error;
-
-    virUUIDFormat(uuid, buf);
+    virUUIDFormat(pool->uuid, buf);
     return 0;
 
 error:
@@ -16843,7 +16830,6 @@ error:
 int
 virSecretGetUUIDString(virSecretPtr secret, char *buf)
 {
-    unsigned char uuid[VIR_UUID_BUFLEN];
     VIR_DEBUG("secret=%p, buf=%p", secret, buf);
 
     virResetLastError();
@@ -16855,10 +16841,7 @@ virSecretGetUUIDString(virSecretPtr secret, char *buf)
     }
     virCheckNonNullArgGoto(buf, error);
 
-    if (virSecretGetUUID(secret, &uuid[0]))
-        goto error;
-
-    virUUIDFormat(uuid, buf);
+    virUUIDFormat(secret->uuid, buf);
     return 0;
 
 error:
@@ -18499,7 +18482,6 @@ error:
 int
 virNWFilterGetUUIDString(virNWFilterPtr nwfilter, char *buf)
 {
-    unsigned char uuid[VIR_UUID_BUFLEN];
     VIR_DEBUG("nwfilter=%p, buf=%p", nwfilter, buf);
 
     virResetLastError();
@@ -18511,10 +18493,7 @@ virNWFilterGetUUIDString(virNWFilterPtr nwfilter, char *buf)
     }
     virCheckNonNullArgGoto(buf, error);
 
-    if (virNWFilterGetUUID(nwfilter, &uuid[0]))
-        goto error;
-
-    virUUIDFormat(uuid, buf);
+    virUUIDFormat(nwfilter->uuid, buf);
     return 0;
 
 error:

diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
index 5334a9f07866d1b08db86d17d32dff9f0ed487ca..7e8ea7f4570c65de02f5bee6ff5634bcec3a955e 100644 (file)
--- a/src/util/virtypedparam.c
+++ b/src/util/virtypedparam.c
@@ -1,7 +1,7 @@
 /*
  * virtypedparam.c: utility functions for dealing with virTypedParameters
  *
- * Copyright (C) 2011-2012 Red Hat, Inc.
+ * Copyright (C) 2011-2014 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -41,6 +41,12 @@ VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_LAST,
               "boolean",
               "string")
 
+/* When editing this file, ensure that public exported functions
+ * (those in libvirt_public.syms) either trigger no errors, or else
+ * reset error on entrance and call virDispatchError() on exit; while
+ * internal utility functions (those in libvirt_private.syms) may
+ * report errors that the caller will dispatch.  */
+
 /* Validate that PARAMS contains only recognized parameter names with
  * correct types, and with no duplicates.  Pass in as many name/type
  * pairs as appropriate, and pass NULL to end the list of accepted
@@ -346,8 +352,6 @@ virTypedParamsReplaceString(virTypedParameterPtr *params,
     size_t n = *nparams;
     virTypedParameterPtr param;
 
-    virResetLastError();
-
     param = virTypedParamsGet(*params, n, name);
     if (param) {
         if (param->type != VIR_TYPED_PARAM_STRING) {
@@ -378,7 +382,6 @@ virTypedParamsReplaceString(virTypedParameterPtr *params,
     return 0;
 
 error:
-    virDispatchError(NULL);
     return -1;
 }
 
@@ -426,6 +429,7 @@ virTypedParamsCopy(virTypedParameterPtr *dst,
  * Finds typed parameter called @name.
  *
  * Returns pointer to the parameter or NULL if it does not exist in @params.
+ * This function does not raise an error, even when returning NULL.
  */
 virTypedParameterPtr
 virTypedParamsGet(virTypedParameterPtr params,
@@ -434,7 +438,7 @@ virTypedParamsGet(virTypedParameterPtr params,
 {
     size_t i;
 
-    virResetLastError();
+    /* No need to reset errors, since this function doesn't report any.  */
 
     if (!params || !name)
         return NULL;
@@ -664,11 +668,11 @@ virTypedParamsGetBoolean(virTypedParameterPtr params,
 {
     virTypedParameterPtr param;
 
+    virResetLastError();
+
     if (!(param = virTypedParamsGet(params, nparams, name)))
         return 0;
 
-    virResetLastError();
-
     VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_BOOLEAN);
     if (value)
         *value = !!param->value.b;