linux-next/include/linux/mtd/pfow.h: possible poor sequence of conditionals ?

linux-next/include/linux/mtd/pfow.h: possible poor sequence of conditionals ?

[David Binderman]

Hello there,

> [linux-next/include/linux/mtd/pfow.h:133]: (style) Expression is always false because 'else if' condition matches previous condition at line 130.
> [linux-next/include/linux/mtd/pfow.h:136]: (style) Expression is always false because 'else if' condition matches previous condition at line 130.

Source code is

    if (prog_status & 0x03)
        printk(KERN_NOTICE"DSR.9,8: (11) Attempt to program invalid "
                        "half with 41h command\n");
    else if (prog_status & 0x02)
        printk(KERN_NOTICE"DSR.9,8: (10) Object Mode Program attempt "
                    "in region with Control Mode data\n");
    else if (prog_status &  0x01)
        printk(KERN_NOTICE"DSR.9,8: (01) Program attempt in region "
                        "with Object Mode data\n");

Maybe something like

    switch (prog_status & 0x03)
    {
        case 0x03:
            printk(); break;
        case 0x02:
            printk(); break;
        case 0x01:
            printk(); break;
    }

might be better code.


Regards

David Binderman

 		 	   		  

Re: linux-next/include/linux/mtd/pfow.h: possible poor sequence of conditionals ?

[Brian Norris]

On Tue, Mar 22, 2016 at 02:49:06PM +0000, David Binderman wrote:
> Hello there,
> 
> > [linux-next/include/linux/mtd/pfow.h:133]: (style) Expression is always false because 'else if' condition matches previous condition at line 130.
> > [linux-next/include/linux/mtd/pfow.h:136]: (style) Expression is always false because 'else if' condition matches previous condition at line 130.
> 
> Source code is
> 
>     if (prog_status & 0x03)
>         printk(KERN_NOTICE"DSR.9,8: (11) Attempt to program invalid "
>                         "half with 41h command\n");
>     else if (prog_status & 0x02)
>         printk(KERN_NOTICE"DSR.9,8: (10) Object Mode Program attempt "
>                     "in region with Control Mode data\n");
>     else if (prog_status &  0x01)
>         printk(KERN_NOTICE"DSR.9,8: (01) Program attempt in region "
>                         "with Object Mode data\n");
> 
> Maybe something like
> 
>     switch (prog_status & 0x03)
>     {
>         case 0x03:
>             printk(); break;
>         case 0x02:
>             printk(); break;
>         case 0x01:
>             printk(); break;
>     }
> 
> might be better code.

I'm not sure if you're aware of this... but this isn't really a style
issue; it's a correctness issue. I believe the original code is wrong.

Feel free to send a patch, especially if you can test it.

Regards,
Brian

RE: linux-next/include/linux/mtd/pfow.h: possible poor sequence of conditionals ?

[David Binderman]

Hello there,

----------------------------------------
> I'm not sure if you're aware of this... but this isn't really a style
> issue; it's a correctness issue. I believe the original code is wrong.

Thanks for the information.

> Feel free to send a patch, especially if you can test it.

Sorry no, I am unable to send a patch. I am happy for someone else
more capable to send in a patch.


Regards

David Binderman