Re: staging/dgap/dgap.c:981: bad if test ?

2015-01-22 Thread Dan Carpenter

On Thu, Jan 22, 2015 at 10:05:53AM +, David Binderman wrote:
 Hello there,
 
 [linux-3.19-rc5/drivers/staging/dgap/dgap.c:981]: (warning) Logical 
 disjunction always evaluates to true: conc_type != 65 || conc_type != 66.
 
 Source code is
 
     if (conc_type == 0 || conc_type != CX ||
     conc_type != EPC) {
 
 Suggest code rework.

Thanks.  This bug was introduced in:

commit f6aa0164cd3b1c5192e87f5651ec382c3bc3abac
Author: Daeseok Youn daeseok.y...@gmail.com
Date:   Sat Aug 9 14:38:41 2014 +0900

staging: dgap: Simplify to set a concentrator type

It is same manner with setting a board type.
For example of config file for concentrator,
conc ccon or
conc epcon

After allocating a type of CNODE then set a type of concentrator.
So remove cases in swith statement, just get a token from string
and set to conc.type. And also it doesn't need to conc.v_type.

Signed-off-by: Daeseok Youn daeseok.y...@gmail.com
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org

Let's CC Daeseok.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: staging/dgap/dgap.c:981: bad if test ?

2015-01-22 Thread David Binderman
Hello there,

Thanks for the confirmation. More of the same in the same file:

[linux-3.19-rc5/drivers/staging/dgap/dgap.c:1022]: (warning) Logical 
disjunction always evaluates to true: module_type != 68 || module_type != 73.

Source code is

    if (module_type == 0 || module_type != PORTS ||
    module_type != MODEM) {

I used the static analyser cppcheck to find these two bugs, but
you might be able to find similar problems by using gcc compiler
flag -Wlogical-op.

Regards

David Binderman



 Date: Thu, 22 Jan 2015 13:14:22 +0300
 From: dan.carpen...@oracle.com
 To: dcb...@hotmail.com; daeseok.y...@gmail.com
 CC: driverdev-devel@linuxdriverproject.org
 Subject: Re: staging/dgap/dgap.c:981: bad if test ?


 On Thu, Jan 22, 2015 at 10:05:53AM +, David Binderman wrote:
 Hello there,

 [linux-3.19-rc5/drivers/staging/dgap/dgap.c:981]: (warning) Logical 
 disjunction always evaluates to true: conc_type != 65 || conc_type != 66.

 Source code is

 if (conc_type == 0 || conc_type != CX ||
 conc_type != EPC) {

 Suggest code rework.

 Thanks. This bug was introduced in:

 commit f6aa0164cd3b1c5192e87f5651ec382c3bc3abac
 Author: Daeseok Youn daeseok.y...@gmail.com
 Date: Sat Aug 9 14:38:41 2014 +0900

 staging: dgap: Simplify to set a concentrator type

 It is same manner with setting a board type.
 For example of config file for concentrator,
 conc ccon or
 conc epcon

 After allocating a type of CNODE then set a type of concentrator.
 So remove cases in swith statement, just get a token from string
 and set to conc.type. And also it doesn't need to conc.v_type.

 Signed-off-by: Daeseok Youn daeseok.y...@gmail.com
 Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org

 Let's CC Daeseok.

 regards,
 dan carpenter

  
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: staging/dgap/dgap.c:981: bad if test ?

2015-01-22 Thread Dan Carpenter
On Thu, Jan 22, 2015 at 10:20:44AM +, David Binderman wrote:
 I used the static analyser cppcheck to find these two bugs, but
 you might be able to find similar problems by using gcc compiler
 flag -Wlogical-op.

I turned it on, but GCC 4.7.2 doesn't find anything for me.

It complains about:
if (x != 1 || x != 2)

but it doesn't complain about:
if (x == 0 || x != 1 || x != 2) 

Smatch is similar to GCC in this regard...

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: staging/dgap/dgap.c:981: bad if test ?

2015-01-22 Thread David Binderman
Hello there,


 On Thu, Jan 22, 2015 at 10:20:44AM +, David Binderman wrote:
 I used the static analyser cppcheck to find these two bugs, but
 you might be able to find similar problems by using gcc compiler
 flag -Wlogical-op.

 I turned it on, but GCC 4.7.2 doesn't find anything for me.

Surprising. 

I note that gcc 4.7.2 is dated Sep 2012 and the current release of gcc is 
4.9.2, dated Oct 2014.
I am not sure upgrading gcc will help.

 Smatch is similar to GCC in this regard...

cppcheck is available from sourceforge.

Regards

David Binderman
  
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: staging/dgap/dgap.c:981: bad if test ?

2015-01-22 Thread DaeSeok Youn
Hi,

2015-01-22 19:14 GMT+09:00 Dan Carpenter dan.carpen...@oracle.com:

 On Thu, Jan 22, 2015 at 10:05:53AM +, David Binderman wrote:
 Hello there,

 [linux-3.19-rc5/drivers/staging/dgap/dgap.c:981]: (warning) Logical 
 disjunction always evaluates to true: conc_type != 65 || conc_type != 66.

 Source code is

 if (conc_type == 0 || conc_type != CX ||
 conc_type != EPC) {

 Suggest code rework.

 Thanks.  This bug was introduced in:
Yes, right. I will fix this bug.

The configuration file has a type of concentrator and that type has
two different modules that are ccon and epc.
If configuration file has another type for concentrator, returns
-1 but it always return an error.

Thanks for reporting.

regards,
Daeseok Youn.


 commit f6aa0164cd3b1c5192e87f5651ec382c3bc3abac
 Author: Daeseok Youn daeseok.y...@gmail.com
 Date:   Sat Aug 9 14:38:41 2014 +0900

 staging: dgap: Simplify to set a concentrator type

 It is same manner with setting a board type.
 For example of config file for concentrator,
 conc ccon or
 conc epcon

 After allocating a type of CNODE then set a type of concentrator.
 So remove cases in swith statement, just get a token from string
 and set to conc.type. And also it doesn't need to conc.v_type.

 Signed-off-by: Daeseok Youn daeseok.y...@gmail.com
 Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org

 Let's CC Daeseok.

 regards,
 dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel