[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 07/24] maint: avoid nested public calls




On 12/28/2013 11:11 AM, Eric Blake wrote:
> 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.
> 
> Signed-off-by: Eric Blake <eblake redhat com>
> ---
>  src/libvirt.c            | 31 +++++--------------------------
>  src/util/virtypedparam.c | 14 +++++++++-----
>  2 files changed, 14 insertions(+), 31 deletions(-)
> 

ACK

Although I do note that virTypedParamsGetBoolean() has a slightly
different call flow than other similar functions...

That is usually it's

virResetLastError()
virTypedParamsGet()
VIR_TYPED_PARAM_CHECK_TYPE()

but, GetBoolean swaps ResetLast and TypedParamsGet()

Since this is a cleanup of sorts - just thought it may be useful to be
consistent.

John


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]