Re: Change in register_blkdev() behavior

From: Logan Gunthorpe
Date: Fri Feb 02 2018 - 01:13:12 EST




On 01/02/18 07:17 PM, Srivatsa S. Bhat wrote:
> Thank you for confirming! I'll send a patch to fix that (and the analogous
> case for CHRDEV_MAJOR_DYN_EXT_END).

Great! Thanks!

>>>
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ for (cd = chrdevs[major_to_index(i)]; cd; cd = cd->next)
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (cd->major == i)
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ break;
>>>
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ if (cd == NULL || cd->major != i)
>>> ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ^^^^^^^^
>>> It seems that this latter condition is unnecessary, as it will never be
>>> true. (We'll reach here only if cd == NULL or cd->major == i).
>>
>> Not quite. chrdevs[] may contain majors that also hit on the hash but don't equal 'i'. So the for loop will iterate through all hashes matching 'i' and if there is one or more and they all don't match 'i', it will fall through the loop and cd will be set to something non-null and not equal to i.
>>
>
> Hmm, the code doesn't appear to be doing that though? The loop's fall
> through occurs one past the last entry, when cd == NULL. The only
> other way it can exit the loop is if it hits the break statement
> (which implies that cd->major == i). So what am I missing?

Oh, yup, you're right. Looking at it a second (or third) time, it should
be NULL or equal to 'i'.

Thanks,

Logan