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
