Re: [PATCH 2/6] idr: remove MAX_IDR_MASK and move left MAX_IDR_*into idr.c

From: Wolfram Sang
Date: Sun Feb 10 2013 - 06:52:25 EST


On Fri, Feb 08, 2013 at 01:01:49PM -0800, Tejun Heo wrote:
> MAX_IDR_MASK is another weirdness in the idr interface. As idr covers
> whole positive integer range, it's defined as 0x7fffffff or INT_MAX.
>
> Its usage in idr_find(), idr_replace() and idr_remove() is bizarre.
> They basically mask off the sign bit and operate on the rest, so if
> the caller, by accident, passes in a negative number, the sign bit
> will be masked off and the remaining part will be used as if that was
> the input, which is worse than crashing.
>
> The constant is visible in idr.h and there are several users in the
> kernel.
>
> * drivers/i2c/i2c-core.c:i2c_add_numbered_adapter()
>
> Basically used to test if adap->nr is a negative number which isn't
> -1 and returns -EINVAL if so. idr_alloc() already has negative
> @start checking (w/ WARN_ON_ONCE), so this can go away.
>
> * drivers/infiniband/core/cm.c:cm_alloc_id()
> drivers/infiniband/hw/mlx4/cm.c:id_map_alloc()
>
> Used to wrap cyclic @start. Can be replaced with max(next, 0).
> Note that this type of cyclic allocation using idr is buggy. These
> are prone to spurious -ENOSPC failure after the first wraparound.
>
> * fs/super.c:get_anon_bdev()
>
> The ID allocated from ida is masked off before being tested whether
> it's inside valid range. ida allocated ID can never be a negative
> number and the masking is unnecessary.
>
> Update idr_*() functions to fail with -EINVAL when negative @id is
> specified and update other MAX_IDR_MASK users as described above.
>
> This leaves MAX_IDR_MASK without any user, remove it and relocate
> other MAX_IDR_* constants to lib/idr.c.
>
> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>

For the i2c-part:

Acked-by: Wolfram Sang <wolfram@xxxxxxxxxxxxx>

--
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/