[PATCH] mtd: Replace if and BUG with BUG_ON
Ezequiel Garcia
ezequiel at vanguardiasur.com.ar
Mon May 30 15:19:00 PDT 2016
Hi Amitoj,
Thanks for your patch.
On 28 May 2016 at 13:41, Amitoj Kaur Chawla <amitoj1606 at gmail.com> wrote:
> Replace if condition and BUG() with a BUG_ON having the conditional
> expression of the if statement as argument.
>
We usually want commit messages that tell us *why* you are doing the
change: what are you fixing, or what are you improving, and what
possible side-effects it may have.
This commit log explains what the code does, but we can clearly see
that, so it's not useful.
> The Coccinelle semantic patch used to make this change is as follows:
> @@ expression E,f; @@
>
> (
> if (<+... f(...) ...+>) { BUG(); }
> |
> - if (E) { BUG(); }
> + BUG_ON(E);
> )
>
> Signed-off-by: Amitoj Kaur Chawla <amitoj1606 at gmail.com>
> ---
> drivers/mtd/ssfdc.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/ssfdc.c b/drivers/mtd/ssfdc.c
> index daf82ba..41b13d1 100644
> --- a/drivers/mtd/ssfdc.c
> +++ b/drivers/mtd/ssfdc.c
> @@ -380,8 +380,7 @@ static int ssfdcr_readsect(struct mtd_blktrans_dev *dev,
> " block_addr=%d\n", logic_sect_no, sectors_per_block, offset,
> block_address);
>
> - if (block_address >= ssfdc->map_len)
> - BUG();
> + BUG_ON(block_address >= ssfdc->map_len);
>
I don't want to be rude, but I wonder if there's any value at all in
such a patch. It barely improves readability, it barely reduces the
LoC, yet it consumes developer time, maintainer time, and changes git
per-line authorship (used in git blame).
I'm not complaining about *this* particular patch, but rather about
these kind of supposedly clean-up patches.
--
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
More information about the linux-mtd
mailing list