Re: [PATCH 6/6] staging: dgap: tty.c: removes smatch warning "unsigned '--un->un_open_count' is never less than zero"

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

On Mon, Sep 23, 2013 at 06:47:17PM -0400, Lidza Louina wrote:
> This patch removes this smatch warning:
> unsigned '--un->un_open_count' is never less than zero
> 
> The code decremented the un_open_count variable
> and tested to see if it was less than zero. Because
> un_open_count is unsigned and can't be below zero,
> this test doesn't work. This patch tests
> un_open_count against 0 without decrementing it.
> 
> Signed-off-by: Lidza Louina <lidza.louina@xxxxxxxxx>
> ---
>  drivers/staging/dgap/dgap_tty.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/dgap/dgap_tty.c b/drivers/staging/dgap/dgap_tty.c
> index 8f0a824..f496710 100644
> --- a/drivers/staging/dgap/dgap_tty.c
> +++ b/drivers/staging/dgap/dgap_tty.c
> @@ -1442,7 +1442,7 @@ static void dgap_tty_close(struct tty_struct *tty, struct file *file)
>  		un->un_open_count = 1;
>  	}  
>  
> -	if (--un->un_open_count < 0) {
> +	if (un->un_open_count == 0) {
>  		APR(("bad serial port open count of %d\n", un->un_open_count));
>  		un->un_open_count = 0;
>  	}

This fix isn't right.  We still need the decrement.  Probably the best
thing is to audit all the driver and make sure that un->un_open_count
can never be == 0.  But the next best, and still totally reasonable
thing to do is this:

	if (un->un_open_count == 0) {
		APR(("bad serial port open count of %d\n", un->un_open_count));
		un->un_open_count = 1;
	}
	un->un_open_count--;

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel