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

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

 




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@xxxxxxxxxx>
> ---
>  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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]