[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADorM-9OnP2zbtn6FHp-3NcO8nfnwC_RbzdcHdiUJCPXyMy8ig@mail.gmail.com>
Date: Tue, 29 Jul 2025 17:40:42 +0530
From: Jogi Dishank <jogidishank503@...il.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: linux-m68k@...ts.linux-m68k.org, linux-kernel@...r.kernel.org,
darshanrathod475@...il.com
Subject: Re: [PATCH] zorro: fix checkpatch error by avoiding assignment in if-statement.
Hi,
Thank you for the suggestion.
Additionally, the conditional logic is inverted to reduce indentation
and improve readability, aligning with kernel coding style.
Suggested-by: Christophe Leroy <christophe.leroy@...roup.eu>
Signed-off-by: Dishank Jogi <jogidishank503@...il.com>
---
drivers/zorro/gen-devlist.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/zorro/gen-devlist.c b/drivers/zorro/gen-devlist.c
index e325c5ce995b..853619ab27df 100644
--- a/drivers/zorro/gen-devlist.c
+++ b/drivers/zorro/gen-devlist.c
@@ -44,31 +44,32 @@ main(void)
while (fgets(line, sizeof(line)-1, stdin)) {
lino++;
- if ((c = strchr(line, '\n')))
+ c = strchr(line, '\n');
+ if (c)
*c = 0;
if (!line[0] || line[0] == '#')
continue;
if (line[0] == '\t') {
switch (mode) {
case 1:
- if (strlen(line) > 5 && line[5] == ' ') {
- c = line + 5;
- while (*c == ' ')
- *c++ = 0;
+ if (strlen(line) <= 5 || line[5] != ' ')
+ goto err;
+ c = line + 5;
+ while (*c == ' ')
+ *c++ = 0;
+ if (manuf_len + strlen(c) + 1 > MAX_NAME_SIZE) {
+ /* Too long, try cutting off
long description */
+ bra = strchr(c, '[');
+ if (bra && bra > c && bra[-1] == ' ')
+ bra[-1] = 0;
if (manuf_len + strlen(c) + 1
> MAX_NAME_SIZE) {
- /* Too long, try
cutting off long description */
- bra = strchr(c, '[');
- if (bra && bra > c &&
bra[-1] == ' ')
- bra[-1] = 0;
- if (manuf_len +
strlen(c) + 1 > MAX_NAME_SIZE) {
-
fprintf(stderr, "Line %d: Product name too long\n", lino);
- return 1;
- }
+ fprintf(stderr, "Line
%d: Product name too long\n", lino);
+ return 1;
}
- fprintf(devf,
"\tPRODUCT(%s,%s,\"", manuf, line+1);
- pq(devf, c);
- fputs("\")\n", devf);
- } else goto err;
+ }
+ fprintf(devf, "\tPRODUCT(%s,%s,\"",
manuf, line+1);
+ pq(devf, c);
+ fputs("\")\n", devf);
break;
default:
goto err;
On Tue, 29 Jul 2025 at 12:38, Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
>
> Hi Dishank,
>
> On Mon, 28 Jul 2025 at 11:34, Dishank Jogi <jogidishank503@...il.com> wrote:
> > These changes improve code readability and bring the file
> > in line with the Linux kernel coding style.
> >
> > No functional changes.
> >
> > Signed-off-by: Dishank Jogi <jogidishank503@...il.com>
>
> Thanks for your patch!
>
> > --- a/drivers/zorro/gen-devlist.c
> > +++ b/drivers/zorro/gen-devlist.c
> > @@ -44,7 +44,8 @@ main(void)
> >
> > while (fgets(line, sizeof(line)-1, stdin)) {
> > lino++;
> > - if ((c = strchr(line, '\n')))
> > + c = strchr(line, '\n')
>
> As Christophe pointed out, this line lacks a semicolon.
> Please do not submit untested patches.
>
> > + if (c)
> > *c = 0;
> > if (!line[0] || line[0] == '#')
> > continue;
> > @@ -68,7 +69,8 @@ main(void)
> > fprintf(devf, "\tPRODUCT(%s,%s,\"", manuf, line+1);
> > pq(devf, c);
> > fputs("\")\n", devf);
> > - } else goto err;
> > + } else
> > + goto err;
>
> This does not comply with Linux kernel coding style: please use braces
> in both branches.
>
> However, in this case I'd rather invert the logic, remove the braces,
> and reduce indentation for the big block:
>
> if (strlen(line) <= 5 || line[5] != ' ')
> goto err;
>
> [...]
>
> > break;
> > default:
> > goto err;
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
Powered by blists - more mailing lists