From: Masahiro Yamada <yamada.masahiro@socionext.com>
To: Heiko Schocher <hs@denx.de>, Richard Weinberger <richard@nod.at>
Cc: linux-mtd@lists.infradead.org,
Artem Bityutskiy <dedekind1@gmail.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] ubifs: compress lines for immediate return
Date: Mon, 5 Sep 2016 21:44:31 +0900 [thread overview]
Message-ID: <CAK7LNARnJVjwDaLGpSUUwvMWSXdXvkaF0H5yV2Dtr=pgpZZwOw@mail.gmail.com> (raw)
In-Reply-To: <1473058466-10831-1-git-send-email-hs@denx.de>
Hi Heiko, Richard,
2016-09-05 15:54 GMT+09:00 Heiko Schocher <hs@denx.de>:
> From: Masahiro Yamada <yamada.masahiro@socionext.com>
>
> Cleanup the following code construct:
> ret = expression;
> if (ret)
> return ret;
> return 0;
>
> into a simple form:
> return expression;
>
> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> posted on the U-Boot mailinglist.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Heiko Schocher <hs@denx.de>
I am the author of the original patch in the U-Boot ML.
Please notice it has not passed the review in U-Boot ML yet.
Actually, I got some feedback against this patch.
See
http://patchwork.ozlabs.org/patch/665199/
Stephan Warren suggested that
we should not break code uniformity.
After I considered it and took a closer look,
I decided that we should not do these changes.
This patch breaks the code uniformity.
See blow:
> /**
> diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c
> index 821b348..88cd61d 100644
> --- a/fs/ubifs/gc.c
> +++ b/fs/ubifs/gc.c
> @@ -297,10 +297,8 @@ static int sort_nodes(struct ubifs_info *c, struct ubifs_scan_leb *sleb,
> err = dbg_check_data_nodes_order(c, &sleb->nodes);
> if (err)
> return err;
> - err = dbg_check_nondata_nodes_order(c, nondata);
> - if (err)
> - return err;
> - return 0;
> +
> + return dbg_check_nondata_nodes_order(c, nondata);
> }
Original code has uniformity here.
err = dbg_check_data_nodes_order(c, &sleb->nodes);
if (err)
return err;
err = dbg_check_nondata_nodes_order(c, nondata);
if (err)
return err;
> /**
> diff --git a/fs/ubifs/lpt_commit.c b/fs/ubifs/lpt_commit.c
> index ce89bdc..79a8e96 100644
> --- a/fs/ubifs/lpt_commit.c
> +++ b/fs/ubifs/lpt_commit.c
> @@ -313,10 +313,7 @@ static int layout_cnodes(struct ubifs_info *c)
> alen = ALIGN(offs, c->min_io_size);
> upd_ltab(c, lnum, c->leb_size - alen, alen - offs);
> dbg_chk_lpt_sz(c, 4, alen - offs);
> - err = dbg_chk_lpt_sz(c, 3, alen);
> - if (err)
> - return err;
> - return 0;
> + return dbg_chk_lpt_sz(c, 3, alen);
>
We have dbg_chk_lpt_sz() call just above (its return value is ignored)
So, returning the value of the last dbg_chk_lpt_sz() call
seems a bit weird. So, I do not want to touch this.
Heiko,
If you want to post this patch, it is up to you.
But, in that case, could you drop my Author and Signed-off-by,
then send it as your patch, please?
I do not feel comfortable with my authorship
for what I decided to not do.
I will retract my original patch from the U-Boot ML, too.
--
Best Regards
Masahiro Yamada
next prev parent reply other threads:[~2016-09-05 12:45 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-05 6:54 [PATCH v2] ubifs: compress lines for immediate return Heiko Schocher
2016-09-05 12:44 ` Masahiro Yamada [this message]
2016-09-05 13:05 ` Heiko Schocher
2016-09-05 13:32 ` Richard Weinberger
2016-09-06 4:37 ` Heiko Schocher
2016-09-05 13:18 ` David Oberhollenzer
2016-09-05 13:26 ` Masahiro Yamada
2016-09-10 23:29 ` kbuild test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAK7LNARnJVjwDaLGpSUUwvMWSXdXvkaF0H5yV2Dtr=pgpZZwOw@mail.gmail.com' \
--to=yamada.masahiro@socionext.com \
--cc=adrian.hunter@intel.com \
--cc=dedekind1@gmail.com \
--cc=hs@denx.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=richard@nod.at \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).