Re: [PATCH] lib/idr.c: remove duplicated bound checking in sub_alloc

[Andrew Morton]

On Thu, 19 Mar 2015 17:57:07 +0000 Liu Sha <rednoax@xxxxxx> wrote:

> From: Liu Sha <rednoax@xxxxxxxxx>
> 
> The INT_MAX bound checking in sub_alloc checks two conditions to see
> whether the signed integer "id" is beyond INT_MAX:
> 
> 		if ((id >= MAX_IDR_BIT) || (id < 0))
> 			return -ENOSPC;
> 
> These two conditions are actually the same for "int" variable so one
> of them can be removed. If the above snippet is compiled with -Os option
> of gcc, only one checking will remain in disassembly code.
> 
> --- a/lib/idr.c
> +++ b/lib/idr.c
> @@ -262,7 +262,7 @@ static int sub_alloc(struct idr *idp, int *starting_id, struct idr_layer **pa,
>  			sh = IDR_BITS*l;
>  			id = ((id >> sh) ^ n ^ m) << sh;
>  		}
> -		if ((id >= MAX_IDR_BIT) || (id < 0))
> +		if (id >= MAX_IDR_BIT)
>  			return -ENOSPC;
>  		if (l == 0)
>  			break;

Well.  This only works because MAX_IDR_BIT happens to have unsigned
type, so the comparison is done with unsigned arithmetic.

The patch makes no difference to code size with my gcc and I'm inclined
to leave the code as-is for reasons of safety and clarity.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/