On 2/1/18 5:10 PM, Logan Gunthorpe wrote:
>
>
> On 01/02/18 05:23 PM, Srivatsa S. Bhat wrote:
>> static int find_dynamic_major(void)
>> {
>> int i;
>> struct char_device_struct *cd;
>>
>> for (i = ARRAY_SIZE(chrdevs)-1; i > CHRDEV_MAJOR_DYN_END; i--) {
>> ^^^^
>> As far as I can see, _DYN_END is inclusive, so shouldn't this be >= ?
>
> Yes, it looks like _DYN_END should have been inclusive based on the way I
> documented it.
>
Thank you for confirming! I'll send a patch to fix that (and the analogous
case for CHRDEV_MAJOR_DYN_EXT_END).
>>
>> 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?
Regards,
Srivatsa
