Re: [PATCH v1 44/59] 9p-local.c: remove unneeded label in local_unlinkat_common()

[Christian Schoenebeck]

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. ;-)

Best regards,
Christian Schoenebeck