| Message ID | 20210801233549.25480-1-mpdesouza@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: send: Simplify send_create_inode_if_needed | expand |
On Mon 02 Aug 2021 at 07:35, Marcos Paulo de Souza <mpdesouza@suse.com> wrote: > The out label is being overused, we can simply return if the > condition > permits. > > No functional changes. > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > Reviewed-by: Su Yue <l@damenly.su> -- Su > --- > fs/btrfs/send.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > index 75cff564dedf..17cd67e41d3a 100644 > --- a/fs/btrfs/send.c > +++ b/fs/btrfs/send.c > @@ -2727,19 +2727,12 @@ static int > send_create_inode_if_needed(struct send_ctx *sctx) > if (S_ISDIR(sctx->cur_inode_mode)) { > ret = did_create_dir(sctx, sctx->cur_ino); > if (ret < 0) > - goto out; > - if (ret) { > - ret = 0; > - goto out; > - } > + return ret; > + if (ret > 0) > + return 0; > } > > - ret = send_create_inode(sctx, sctx->cur_ino); > - if (ret < 0) > - goto out; > - > -out: > - return ret; > + return send_create_inode(sctx, sctx->cur_ino); > } > > struct recorded_ref {
On 2.08.21 г. 2:35, Marcos Paulo de Souza wrote: > The out label is being overused, we can simply return if the condition > permits. > > No functional changes. > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/send.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > index 75cff564dedf..17cd67e41d3a 100644 > --- a/fs/btrfs/send.c > +++ b/fs/btrfs/send.c > @@ -2727,19 +2727,12 @@ static int send_create_inode_if_needed(struct send_ctx *sctx) > if (S_ISDIR(sctx->cur_inode_mode)) { > ret = did_create_dir(sctx, sctx->cur_ino); > if (ret < 0) > - goto out; > - if (ret) { > - ret = 0; > - goto out; > - } > + return ret; > + if (ret > 0) > + return 0; nit: Personally I'd prefer in such cases to use an if/else if construct since which branch is taken is dependent on the same value. To me using an if/else if is a more explicit way to say "those 2 branches are directly related). However it's not a big deal. > } > > - ret = send_create_inode(sctx, sctx->cur_ino); > - if (ret < 0) > - goto out; > - > -out: > - return ret; > + return send_create_inode(sctx, sctx->cur_ino); > } > > struct recorded_ref { >
On Mon, 2021-08-02 at 10:39 +0300, Nikolay Borisov wrote: > > On 2.08.21 г. 2:35, Marcos Paulo de Souza wrote: > > The out label is being overused, we can simply return if the > > condition > > permits. > > > > No functional changes. > > > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > > Reviewed-by: Nikolay Borisov <nborisov@suse.com> > > --- > > fs/btrfs/send.c | 15 ++++----------- > > 1 file changed, 4 insertions(+), 11 deletions(-) > > > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > > index 75cff564dedf..17cd67e41d3a 100644 > > --- a/fs/btrfs/send.c > > +++ b/fs/btrfs/send.c > > @@ -2727,19 +2727,12 @@ static int > > send_create_inode_if_needed(struct send_ctx *sctx) > > if (S_ISDIR(sctx->cur_inode_mode)) { > > ret = did_create_dir(sctx, sctx->cur_ino); > > if (ret < 0) > > - goto out; > > - if (ret) { > > - ret = 0; > > - goto out; > > - } > > + return ret; > > + if (ret > 0) > > + return 0; > > nit: Personally I'd prefer in such cases to use an if/else if > construct > since which branch is taken is dependent on the same value. To me > using > an if/else if is a more explicit way to say "those 2 branches are > directly related). However it's not a big deal. Indeed, it's better. Should I send a v2 in this case or david can just add an 'else' to the last if clause? Thanks > > > } > > > > - ret = send_create_inode(sctx, sctx->cur_ino); > > - if (ret < 0) > > - goto out; > > - > > -out: > > - return ret; > > + return send_create_inode(sctx, sctx->cur_ino); > > } > > > > struct recorded_ref { > >
On Sun, Aug 01, 2021 at 08:35:49PM -0300, Marcos Paulo de Souza wrote: > The out label is being overused, we can simply return if the condition > permits. > > No functional changes. > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> Added to misc-next, with the if-else update, thanks.
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 75cff564dedf..17cd67e41d3a 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -2727,19 +2727,12 @@ static int send_create_inode_if_needed(struct send_ctx *sctx) if (S_ISDIR(sctx->cur_inode_mode)) { ret = did_create_dir(sctx, sctx->cur_ino); if (ret < 0) - goto out; - if (ret) { - ret = 0; - goto out; - } + return ret; + if (ret > 0) + return 0; } - ret = send_create_inode(sctx, sctx->cur_ino); - if (ret < 0) - goto out; - -out: - return ret; + return send_create_inode(sctx, sctx->cur_ino); } struct recorded_ref {
The out label is being overused, we can simply return if the condition permits. No functional changes. Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> --- fs/btrfs/send.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)