[libvirt] [PATCH 2/6] qemu_conf: Drop a pair of needless 'cleanup' labels
Pavel Hrdina
phrdina at redhat.com
Tue Sep 10 07:19:50 UTC 2019
On Mon, Sep 09, 2019 at 06:00:23PM +0200, Michal Privoznik wrote:
> There are two 'cleanup' labels - one in
> virQEMUDriverConfigHugeTLBFSInit() and the other in
> virQEMUDriverConfigSetDefaults() that do nothing more than
> return and integer value. No memory freeing or anything important
> is done there. Drop them in favour of returning immediately.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> src/qemu/qemu_conf.c | 26 ++++++++------------------
> 1 file changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 32d411e536..f11df03cf8 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -402,18 +402,12 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
> const char *path,
> bool deflt)
> {
> - int ret = -1;
> -
> - if (VIR_STRDUP(hugetlbfs->mnt_dir, path) < 0)
> - goto cleanup;
> -
> - if (virFileGetHugepageSize(path, &hugetlbfs->size) < 0)
> - goto cleanup;
> + if (VIR_STRDUP(hugetlbfs->mnt_dir, path) < 0 ||
> + virFileGetHugepageSize(path, &hugetlbfs->size) < 0)
> + return -1;
I prefer the old code-style to have two separate if conditions but
that's just my personal view.
We don't have a syntax-check rule for that but the body should be
wrapped in curly braces [1]. I know that our code doesn't follow that
rule but we should avoid introducing new occurrences.
>
> hugetlbfs->deflt = deflt;
> - ret = 0;
> - cleanup:
> - return ret;
> + return 0;
> }
>
>
> @@ -1172,8 +1166,6 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
> int
> virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg)
> {
> - int ret = -1;
> -
> #define SET_TLS_SECRET_UUID_DEFAULT(val) \
> do { \
> if (!cfg->val## TLSx509certdir && \
> @@ -1181,7 +1173,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg)
> cfg->defaultTLSx509secretUUID) { \
> if (VIR_STRDUP(cfg->val## TLSx509secretUUID, \
> cfg->defaultTLSx509secretUUID) < 0) \
> - goto cleanup; \
> + return -1; \
> } \
> } while (0)
>
> @@ -1205,11 +1197,11 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg)
> if (virFileExists(SYSCONFDIR "/pki/libvirt-"#val)) { \
> if (VIR_STRDUP(cfg->val ## TLSx509certdir, \
> SYSCONFDIR "/pki/libvirt-"#val) < 0) \
> - goto cleanup; \
> + return -1; \
> } else { \
> if (VIR_STRDUP(cfg->val ## TLSx509certdir, \
> cfg->defaultTLSx509certdir) < 0) \
> - goto cleanup; \
> + return -1; \
I would fix all the cases above to use curly-braces but it's a existing
code so I'll leave that up to you.
Reviewed-by: Pavel Hrdina <phrdina at redhat.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190910/8194656f/attachment-0001.sig>
More information about the libvir-list
mailing list