[PATCH] zorro: fix checkpatch error by avoiding assignment in if-statement.

[PATCH] zorro: fix checkpatch error by avoiding assignment in if-statement.

[Dishank Jogi]

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@gmail.com>
---
 drivers/zorro/gen-devlist.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/zorro/gen-devlist.c b/drivers/zorro/gen-devlist.c
index e325c5ce995b..ff4515e02409 100644
--- 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')
+		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;
 				break;
 			default:
 				goto err;
-- 
2.43.0

Re: [PATCH] zorro: fix checkpatch error by avoiding assignment in if-statement.

[Christophe JAILLET]

Le 28/07/2025 à 11:34, Dishank Jogi a écrit :
> 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@gmail.com>
> ---
>   drivers/zorro/gen-devlist.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/zorro/gen-devlist.c b/drivers/zorro/gen-devlist.c
> index e325c5ce995b..ff4515e02409 100644
> --- 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')

Looks like a trailing ; is missing.

Was this patch compile-tested?

No change is trivial, even if they look so.
Please always compile test your changes.

CJ

> +		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;
>   				break;
>   			default:
>   				goto err;

Re: [PATCH] zorro: fix checkpatch error by avoiding assignment in if-statement.

[Geert Uytterhoeven]

Hi Dishank,

On Mon, 28 Jul 2025 at 11:34, Dishank Jogi <jogidishank503@gmail.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@gmail.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@linux-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

Re: [PATCH] zorro: fix checkpatch error by avoiding assignment in if-statement.

[Jogi Dishank]

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@csgroup.eu>
Signed-off-by: Dishank Jogi <jogidishank503@gmail.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@linux-m68k.org> wrote:
>
> Hi Dishank,
>
> On Mon, 28 Jul 2025 at 11:34, Dishank Jogi <jogidishank503@gmail.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@gmail.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@linux-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

Re: [PATCH] zorro: fix checkpatch error by avoiding assignment in if-statement.

[Geert Uytterhoeven]

Hi Jogi,

On Tue, 29 Jul 2025 at 14:10, Jogi Dishank <jogidishank503@gmail.com> wrote:
>
> Hi,
>
> Thank you for the suggestion.
>
> Additionally, the conditional logic is inverted to reduce indentation
> and improve readability, aligning with kernel coding style.

Thanks, but this is not how you send a v2.
Please read Documentation/process/submitting-patches.rst.

> Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>

Unless I am mistaken, Christophe merely pointed out a bug in
your first submission, he did not suggest this patch.

> Signed-off-by: Dishank Jogi <jogidishank503@gmail.com>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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

Re: [PATCH] zorro: fix checkpatch error by avoiding assignment in if-statement.

[kernel test robot]

Hi Dishank,

kernel test robot noticed the following build errors:

[auto build test ERROR on geert-m68k/for-next]
[also build test ERROR on gerg-m68knommu/for-next geert-m68k/for-linus linus/master v6.16 next-20250729]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dishank-Jogi/zorro-fix-checkpatch-error-by-avoiding-assignment-in-if-statement/20250728-173702
base:   https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git for-next
patch link:    https://lore.kernel.org/r/20250728093412.48065-1-jogidishank503%40gmail.com
patch subject: [PATCH] zorro: fix checkpatch error by avoiding assignment in if-statement.
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20250729/202507292343.vsRhsbLu-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250729/202507292343.vsRhsbLu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507292343.vsRhsbLu-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/zorro/gen-devlist.c: In function 'main':
>> drivers/zorro/gen-devlist.c:47:39: error: expected ';' before 'if'
      47 |                 c = strchr(line, '\n')
         |                                       ^
         |                                       ;
      48 |                 if (c)
         |                 ~~                     


vim +47 drivers/zorro/gen-devlist.c

    28	
    29	int
    30	main(void)
    31	{
    32		char line[1024], *c, *bra, manuf[8];
    33		int manufs = 0;
    34		int mode = 0;
    35		int lino = 0;
    36		int manuf_len = 0;
    37		FILE *devf;
    38	
    39		devf = fopen("devlist.h", "w");
    40		if (!devf) {
    41			fprintf(stderr, "Cannot create output file!\n");
    42			return 1;
    43		}
    44	
    45		while (fgets(line, sizeof(line)-1, stdin)) {
    46			lino++;
  > 47			c = strchr(line, '\n')

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki