On Tue, Sep 10, 2019 at 09:19:50AM +0200, Pavel Hrdina wrote:
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@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.
Ups, missed the link to hacking guide. [1] <https://libvirt.org/hacking.html#curly_braces>