[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 44/59] 9p-local.c: remove unneeded label in local_unlinkat
From: |
Greg Kurz |
Subject: |
Re: [PATCH v1 44/59] 9p-local.c: remove unneeded label in local_unlinkat_common() |
Date: |
Tue, 7 Jan 2020 14:58:53 +0100 |
On Tue, 07 Jan 2020 13:04:20 +0100
Christian Schoenebeck <address@hidden> wrote:
> On Montag, 6. Januar 2020 19:24:10 CET Daniel Henrique Barboza wrote:
> > 'err_out' can be replaced by 'return ret' in the error conditions
> > the jump was being made.
> >
> > CC: Greg Kurz <address@hidden>
> > Signed-off-by: Daniel Henrique Barboza <address@hidden>
> > ---
> > hw/9pfs/9p-local.c | 12 +++++-------
> > 1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> > index ca641390fb..f9bdd2ad7c 100644
> > --- a/hw/9pfs/9p-local.c
> > +++ b/hw/9pfs/9p-local.c
> > @@ -1094,12 +1094,12 @@ static int local_unlinkat_common(FsContext *ctx, int
> > dirfd, const char *name,
> >
> > fd = openat_dir(dirfd, name);
> > if (fd == -1) {
> > - goto err_out;
> > + return ret;
> > }
> > ret = unlinkat(fd, VIRTFS_META_DIR, AT_REMOVEDIR);
> > close_preserve_errno(fd);
> > if (ret < 0 && errno != ENOENT) {
> > - goto err_out;
> > + return ret;
> > }
> > }
> > map_dirfd = openat_dir(dirfd, VIRTFS_META_DIR);
> > @@ -1107,16 +1107,14 @@ static int local_unlinkat_common(FsContext *ctx, int
> > dirfd, const char *name, ret = unlinkat(map_dirfd, name, 0);
> > close_preserve_errno(map_dirfd);
> > if (ret < 0 && errno != ENOENT) {
> > - goto err_out;
> > + return ret;
> > }
> > } else if (errno != ENOENT) {
> > - goto err_out;
> > + return ret;
> > }
> > }
> >
> > - ret = unlinkat(dirfd, name, flags);
> > -err_out:
> > - return ret;
> > + return unlinkat(dirfd, name, flags);
> > }
> >
> > static int local_remove(FsContext *ctx, const char *path)
>
> Well, personally I don't see any improvement by these changes. It probably
> makes the code slightly more elegant, but IMO not more readable. And return
> constructed functions vs. jump to label constructed functions are more likely
> to gather missing-cleanup bugs.
>
> At least this patch does not cause any behaviour change, so I leave that up
> to
> you Greg to decide. ;-)
>
I don't care that much but in the present case, the function has a bug that
can be fixed with a variant of this patch if 'goto err_out' is turned into
'return -1'. I've hence asked Daniel to move this patch out of the series and
to turn it into a real fix that I'll merge directly.
> Best regards,
> Christian Schoenebeck
>
>
- [PATCH v1 40/59] hsb/hcd-ehci.c: remove unneeded labels, (continued)
- [PATCH v1 40/59] hsb/hcd-ehci.c: remove unneeded labels, Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 41/59] intc/s390_flic_kvm.c: remove unneeded label in kvm_flic_load(), Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 42/59] i386/intel_iommu.c: remove unneeded labels, Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 43/59] i386/amd_iommu.c: remove unneeded label in amdvi_int_remap_msi(), Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 44/59] 9p-local.c: remove unneeded label in local_unlinkat_common(), Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 45/59] 9pfs/9p.c: remove unneeded labels, Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 46/59] alpha/typhoon.c: remove unneeded label in typhoon_translate_iommu(), Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 47/59] pvrdma_main.c: remove unneeded labels, Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 48/59] pvrdma_dev_ring.c: remove unneeded label in pvrdma_ring_init(), Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 49/59] rdma/rdma_rm.c: remove unneeded label in rdma_rm_alloc_pd(), Daniel Henrique Barboza, 2020/01/06
- [PATCH v1 50/59] rdma/rdma_backend.c: remove unneeded label in rdma_backend_init(), Daniel Henrique Barboza, 2020/01/06