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>
This commit is contained in:
Eric Blake 2013-12-27 20:26:03 -07:00
parent d69415d4bc
commit 8f6c845f17
2 changed files with 16 additions and 33 deletions

View File

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

View File

@ -1,7 +1,7 @@
/* /*
* virtypedparam.c: utility functions for dealing with virTypedParameters * 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 * This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public * modify it under the terms of the GNU Lesser General Public
@ -41,6 +41,12 @@ VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_LAST,
"boolean", "boolean",
"string") "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 /* Validate that PARAMS contains only recognized parameter names with
* correct types, and with no duplicates. Pass in as many name/type * correct types, and with no duplicates. Pass in as many name/type
* pairs as appropriate, and pass NULL to end the list of accepted * pairs as appropriate, and pass NULL to end the list of accepted
@ -346,8 +352,6 @@ virTypedParamsReplaceString(virTypedParameterPtr *params,
size_t n = *nparams; size_t n = *nparams;
virTypedParameterPtr param; virTypedParameterPtr param;
virResetLastError();
param = virTypedParamsGet(*params, n, name); param = virTypedParamsGet(*params, n, name);
if (param) { if (param) {
if (param->type != VIR_TYPED_PARAM_STRING) { if (param->type != VIR_TYPED_PARAM_STRING) {
@ -378,7 +382,6 @@ virTypedParamsReplaceString(virTypedParameterPtr *params,
return 0; return 0;
error: error:
virDispatchError(NULL);
return -1; return -1;
} }
@ -426,6 +429,7 @@ virTypedParamsCopy(virTypedParameterPtr *dst,
* Finds typed parameter called @name. * Finds typed parameter called @name.
* *
* Returns pointer to the parameter or NULL if it does not exist in @params. * 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 virTypedParameterPtr
virTypedParamsGet(virTypedParameterPtr params, virTypedParamsGet(virTypedParameterPtr params,
@ -434,7 +438,7 @@ virTypedParamsGet(virTypedParameterPtr params,
{ {
size_t i; size_t i;
virResetLastError(); /* No need to reset errors, since this function doesn't report any. */
if (!params || !name) if (!params || !name)
return NULL; return NULL;
@ -664,11 +668,11 @@ virTypedParamsGetBoolean(virTypedParameterPtr params,
{ {
virTypedParameterPtr param; virTypedParameterPtr param;
virResetLastError();
if (!(param = virTypedParamsGet(params, nparams, name))) if (!(param = virTypedParamsGet(params, nparams, name)))
return 0; return 0;
virResetLastError();
VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_BOOLEAN); VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_BOOLEAN);
if (value) if (value)
*value = !!param->value.b; *value = !!param->value.b;