* [PATCH v2] ubifs: compress lines for immediate return
@ 2016-09-05 6:54 Heiko Schocher
2016-09-05 12:44 ` Masahiro Yamada
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Heiko Schocher @ 2016-09-05 6:54 UTC (permalink / raw)
To: linux-mtd
Cc: Masahiro Yamada, Heiko Schocher, Artem Bityutskiy,
Richard Weinberger, Adrian Hunter, linux-kernel
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>
---
Changes in v2:
- add comment from Richard Weinberger:
rephrase commit message
add Masahiros "Signed-off-by" tag.
fs/ubifs/budget.c | 7 ++-----
fs/ubifs/gc.c | 6 ++----
fs/ubifs/lpt_commit.c | 5 +----
3 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
index 11a11b3..48d6851 100644
--- a/fs/ubifs/budget.c
+++ b/fs/ubifs/budget.c
@@ -77,7 +77,7 @@ static void shrink_liability(struct ubifs_info *c, int nr_to_write)
*/
static int run_gc(struct ubifs_info *c)
{
- int err, lnum;
+ int lnum;
/* Make some free space by garbage-collecting dirty space */
down_read(&c->commit_sem);
@@ -88,10 +88,7 @@ static int run_gc(struct ubifs_info *c)
/* GC freed one LEB, return it to lprops */
dbg_budg("GC freed LEB %d", lnum);
- err = ubifs_return_leb(c, lnum);
- if (err)
- return err;
- return 0;
+ return = ubifs_return_leb(c, lnum);
}
/**
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);
}
/**
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);
no_space:
ubifs_err(c, "LPT out of space at LEB %d:%d needing %d, done_ltab %d, done_lsave %d",
--
2.5.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ubifs: compress lines for immediate return
2016-09-05 6:54 [PATCH v2] ubifs: compress lines for immediate return Heiko Schocher
@ 2016-09-05 12:44 ` Masahiro Yamada
2016-09-05 13:05 ` Heiko Schocher
2016-09-05 13:18 ` David Oberhollenzer
2016-09-10 23:29 ` kbuild test robot
2 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2016-09-05 12:44 UTC (permalink / raw)
To: Heiko Schocher, Richard Weinberger
Cc: linux-mtd, Artem Bityutskiy, Adrian Hunter, Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ubifs: compress lines for immediate return
2016-09-05 12:44 ` Masahiro Yamada
@ 2016-09-05 13:05 ` Heiko Schocher
2016-09-05 13:32 ` Richard Weinberger
0 siblings, 1 reply; 8+ messages in thread
From: Heiko Schocher @ 2016-09-05 13:05 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Richard Weinberger, linux-mtd, Artem Bityutskiy, Adrian Hunter,
Linux Kernel Mailing List
Hello Masahiro,
Am 05.09.2016 um 14:44 schrieb Masahiro Yamada:
> 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.
Oh, then I was a little to fast ... sorry.
@Richard: Should we just forget this patch?
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ubifs: compress lines for immediate return
2016-09-05 6:54 [PATCH v2] ubifs: compress lines for immediate return Heiko Schocher
2016-09-05 12:44 ` Masahiro Yamada
@ 2016-09-05 13:18 ` David Oberhollenzer
2016-09-05 13:26 ` Masahiro Yamada
2016-09-10 23:29 ` kbuild test robot
2 siblings, 1 reply; 8+ messages in thread
From: David Oberhollenzer @ 2016-09-05 13:18 UTC (permalink / raw)
To: Heiko Schocher, linux-mtd
Cc: Artem Bityutskiy, Richard Weinberger, Adrian Hunter,
linux-kernel, Masahiro Yamada
On 09/05/2016 08:54 AM, Heiko Schocher wrote:
> diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
> index 11a11b3..48d6851 100644
> --- a/fs/ubifs/budget.c
> +++ b/fs/ubifs/budget.c
> @@ -77,7 +77,7 @@ static void shrink_liability(struct ubifs_info *c, int nr_to_write)
> */
> static int run_gc(struct ubifs_info *c)
> {
> - int err, lnum;
> + int lnum;
>
> /* Make some free space by garbage-collecting dirty space */
> down_read(&c->commit_sem);
> @@ -88,10 +88,7 @@ static int run_gc(struct ubifs_info *c)
>
> /* GC freed one LEB, return it to lprops */
> dbg_budg("GC freed LEB %d", lnum);
> - err = ubifs_return_leb(c, lnum);
> - if (err)
> - return err;
> - return 0;
> + return = ubifs_return_leb(c, lnum);
> }
>
Apart from the other issues discussed below and in v1, I don't
believe that this _ever_ compiled successfully.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ubifs: compress lines for immediate return
2016-09-05 13:18 ` David Oberhollenzer
@ 2016-09-05 13:26 ` Masahiro Yamada
0 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2016-09-05 13:26 UTC (permalink / raw)
To: David Oberhollenzer
Cc: Heiko Schocher, linux-mtd, Artem Bityutskiy, Richard Weinberger,
Adrian Hunter, Linux Kernel Mailing List
2016-09-05 22:18 GMT+09:00 David Oberhollenzer
<david.oberhollenzer@sigma-star.at>:
> On 09/05/2016 08:54 AM, Heiko Schocher wrote:
>> diff --git a/fs/ubifs/budget.c b/fs/ubifs/budget.c
>> index 11a11b3..48d6851 100644
>> --- a/fs/ubifs/budget.c
>> +++ b/fs/ubifs/budget.c
>> @@ -77,7 +77,7 @@ static void shrink_liability(struct ubifs_info *c, int nr_to_write)
>> */
>> static int run_gc(struct ubifs_info *c)
>> {
>> - int err, lnum;
>> + int lnum;
>>
>> /* Make some free space by garbage-collecting dirty space */
>> down_read(&c->commit_sem);
>> @@ -88,10 +88,7 @@ static int run_gc(struct ubifs_info *c)
>>
>> /* GC freed one LEB, return it to lprops */
>> dbg_budg("GC freed LEB %d", lnum);
>> - err = ubifs_return_leb(c, lnum);
>> - if (err)
>> - return err;
>> - return 0;
>> + return = ubifs_return_leb(c, lnum);
>> }
>>
>
> Apart from the other issues discussed below and in v1, I don't
> believe that this _ever_ compiled successfully.
>
Just in case:
It was not me who added '=' after 'return'.
(I usually run build-test before sending my patches.)
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ubifs: compress lines for immediate return
2016-09-05 13:05 ` Heiko Schocher
@ 2016-09-05 13:32 ` Richard Weinberger
2016-09-06 4:37 ` Heiko Schocher
0 siblings, 1 reply; 8+ messages in thread
From: Richard Weinberger @ 2016-09-05 13:32 UTC (permalink / raw)
To: hs, Masahiro Yamada
Cc: linux-mtd, Artem Bityutskiy, Adrian Hunter, Linux Kernel Mailing List
On 05.09.2016 15:05, Heiko Schocher wrote:
> @Richard: Should we just forget this patch?
Let's drop it for now.
It caused already a way more churn than a trivial style cleanup
patch should...
Thanks,
//richard
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ubifs: compress lines for immediate return
2016-09-05 13:32 ` Richard Weinberger
@ 2016-09-06 4:37 ` Heiko Schocher
0 siblings, 0 replies; 8+ messages in thread
From: Heiko Schocher @ 2016-09-06 4:37 UTC (permalink / raw)
To: Richard Weinberger
Cc: Masahiro Yamada, linux-mtd, Artem Bityutskiy, Adrian Hunter,
Linux Kernel Mailing List
Hello Richard,
Am 05.09.2016 um 15:32 schrieb Richard Weinberger:
> On 05.09.2016 15:05, Heiko Schocher wrote:
>> @Richard: Should we just forget this patch?
>
> Let's drop it for now.
> It caused already a way more churn than a trivial style cleanup
> patch should...
Yes! It was a too fast shoot ... sorry for the noise!
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ubifs: compress lines for immediate return
2016-09-05 6:54 [PATCH v2] ubifs: compress lines for immediate return Heiko Schocher
2016-09-05 12:44 ` Masahiro Yamada
2016-09-05 13:18 ` David Oberhollenzer
@ 2016-09-10 23:29 ` kbuild test robot
2 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2016-09-10 23:29 UTC (permalink / raw)
To: Heiko Schocher
Cc: kbuild-all, linux-mtd, Masahiro Yamada, Heiko Schocher,
Artem Bityutskiy, Richard Weinberger, Adrian Hunter,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1516 bytes --]
Hi Masahiro,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.8-rc5 next-20160909]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Heiko-Schocher/ubifs-compress-lines-for-immediate-return/20160905-145802
config: i386-randconfig-x0-09110426 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
fs/ubifs/budget.c: In function 'run_gc':
>> fs/ubifs/budget.c:91:9: error: expected expression before '=' token
return = ubifs_return_leb(c, lnum);
^
fs/ubifs/budget.c:92:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
vim +91 fs/ubifs/budget.c
85 up_read(&c->commit_sem);
86 if (lnum < 0)
87 return lnum;
88
89 /* GC freed one LEB, return it to lprops */
90 dbg_budg("GC freed LEB %d", lnum);
> 91 return = ubifs_return_leb(c, lnum);
92 }
93
94 /**
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 21949 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-10 23:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-05 6:54 [PATCH v2] ubifs: compress lines for immediate return Heiko Schocher
2016-09-05 12:44 ` Masahiro Yamada
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
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.