lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ