[PATCH 04/10] staging: unisys: fix all logical continuation virthba

Dan Carpenter dan.carpenter at oracle.com
Tue Oct 28 06:46:31 UTC 2014


On Mon, Oct 27, 2014 at 05:14:02PM -0400, Erik Arfvidson wrote:
> This patch fixes all logical continuations issues in virthba.c
> 
> Signed-off-by: Erik Arfvidson <erik.arfvidson at unisys.com>
> ---
>  drivers/staging/unisys/virthba/virthba.c | 42 ++++++++++++++++----------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/staging/unisys/virthba/virthba.c b/drivers/staging/unisys/virthba/virthba.c
> index 6c0ee48..73a7307 100644
> --- a/drivers/staging/unisys/virthba/virthba.c
> +++ b/drivers/staging/unisys/virthba/virthba.c
> @@ -431,10 +431,10 @@ virthba_ISR(int irq, void *dev_id)
>  	virthbainfo->interrupts_rcvd++;
>  	pChannelHeader = virthbainfo->chinfo.queueinfo->chan;
>  	if (((readq(&pChannelHeader->Features)
> -	      & ULTRA_IO_IOVM_IS_OK_WITH_DRIVER_DISABLING_INTS) != 0)
> -	    && ((readq(&pChannelHeader->Features) &
> -		 ULTRA_IO_DRIVER_DISABLES_INTS) !=
> -		0)) {
> +	      &ULTRA_IO_IOVM_IS_OK_WITH_DRIVER_DISABLING_INTS) != 0) &

This isn't right.

"&foo" means address.
"& foo" means bitwise AND.

"!= 0" doesn't add anything here.  When you have "if (strcmp(a, b) != 0)"
that's idiomatic and the != means "a != b".  When you are talking about
the number zero then not equal to zero can be appropriate.  But in this
case it's just a double negative that makes the code slightly not easier
to understand.

The "ULTRA_IO_IOVM_IS_OK_WITH_DRIVER_DISABLING_INTS" is clown shoes
name.  It's silly long.

Also the last & should be a logical &&.

You can fix this stuff up in a later patch since it doesn't have to do
with whitespace.

TODO-list: 2014-10-28: unisys: various style issues

regards,
dan carpenter



More information about the devel mailing list