Re: [PATCH] inotify: Extend ioctl to allow to request id of new watch descriptor

[Kirill Tkhai <ktkhai@virtuozzo.com>]

On 08.02.2018 19:02, Matthew Wilcox wrote:
> On Thu, Feb 08, 2018 at 06:07:37PM +0300, Kirill Tkhai wrote:
>> The new code is under CONFIG_CHECKPOINT_RESTORE #define, so small system
>> may exclude it. The only change in generic inotify part is idr_alloc_cyclic()
>> end argument. We had 0 there, and idr subsystem replaced it with INT_MAX
>> in idr_get_free(). So, the max possible id was INT_MAX (see idr_get_free()
>> again).
>>
>> Since I need INOTIFY_IDR_END to check ioctl's third argument, it's better
>> it's defined as positive number. But when not-zero value is passed
>> to idr_get_free(), this function decrements it. Also, idr_alloc_cyclic()
>> defined @end as int argument. So, it's impossible to pass positive @end
>> argument to idr_alloc_cyclic() to get INT_MAX id. And after this patch
>> inotify watch descriptors ids will take numbers [1, INT_MAX-1], INT_MAX
>> will be unavailable.
> 
> Ummm.  Why not just do:
> 
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +	case INOTIFY_IOC_SETNEXTWD:
> +		ret = -EINVAL;
> +		if (arg >= 1 && arg <= INT_MAX) {
> +			spin_lock(&data->idr_lock);
> +			idr_set_cursor(&data->idr, (unsigned int)arg);
> +			spin_unlock(&data->idr_lock);
> +			ret = 0;
> +		}
> +		break;
> +#endif /* CONFIG_CHECKPOINT_RESTORE */

INT_MAX is generic constant, and the fact, it is used in the above hunk,
does not make feel it's the same constant as used in inotify_add_to_idr().

Specific constant underlines these two places are connected, and it makes
the code more readable for a reader.

Kirill