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 <mpriv...@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>

Attachment: signature.asc
Description: PGP signature

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

Reply via email to