diff mbox series

kdev_t: mask mi with MINORMASK in MKDEV macro

Message ID 5d384dcb-5590-60f8-a4e1-efa6b8da151f@huawei.com (mailing list archive)
State New, archived
Headers show
Series kdev_t: mask mi with MINORMASK in MKDEV macro | expand

Commit Message

Zhiqiang Liu Jan. 10, 2020, 6:37 a.m. UTC
In MKDEV macro, if mi is larger than MINORMASK, the major will be
affected by mi. For example, set dev = MKDEV(2, (1U << MINORBITS)),
then MAJOR(dev) will be equal to 3, incorrectly.

Here, we mask mi with MINORMASK in MKDEV macro.

Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
 include/linux/kdev_t.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bart Van Assche Jan. 11, 2020, 4:50 a.m. UTC | #1
On 2020-01-09 22:37, Zhiqiang Liu wrote:
> 
> In MKDEV macro, if mi is larger than MINORMASK, the major will be
> affected by mi. For example, set dev = MKDEV(2, (1U << MINORBITS)),
> then MAJOR(dev) will be equal to 3, incorrectly.
> 
> Here, we mask mi with MINORMASK in MKDEV macro.
> 
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> ---
>  include/linux/kdev_t.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/kdev_t.h b/include/linux/kdev_t.h
> index 85b5151911cf..40a9423720b2 100644
> --- a/include/linux/kdev_t.h
> +++ b/include/linux/kdev_t.h
> @@ -9,7 +9,7 @@
> 
>  #define MAJOR(dev)	((unsigned int) ((dev) >> MINORBITS))
>  #define MINOR(dev)	((unsigned int) ((dev) & MINORMASK))
> -#define MKDEV(ma,mi)	(((ma) << MINORBITS) | (mi))
> +#define MKDEV(ma, mi)	(((ma) << MINORBITS) | ((mi) & MINORMASK))
> 
>  #define print_dev_t(buffer, dev)					\
>  	sprintf((buffer), "%u:%u\n", MAJOR(dev), MINOR(dev))

Shouldn't the users of MKDEV() be fixed instead of changing the MKDEV()
definition?

Thanks,

Bart.
Zhiqiang Liu Jan. 12, 2020, 8:21 a.m. UTC | #2
On 2020/1/11 12:50, Bart Van Assche wrote:
> On 2020-01-09 22:37, Zhiqiang Liu wrote:
>>
>> In MKDEV macro, if mi is larger than MINORMASK, the major will be
>> affected by mi. For example, set dev = MKDEV(2, (1U << MINORBITS)),
>> then MAJOR(dev) will be equal to 3, incorrectly.
>>
>> Here, we mask mi with MINORMASK in MKDEV macro.
>>
>> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
>> ---
>>  include/linux/kdev_t.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/kdev_t.h b/include/linux/kdev_t.h
>> index 85b5151911cf..40a9423720b2 100644
>> --- a/include/linux/kdev_t.h
>> +++ b/include/linux/kdev_t.h
>> @@ -9,7 +9,7 @@
>>
>>  #define MAJOR(dev)	((unsigned int) ((dev) >> MINORBITS))
>>  #define MINOR(dev)	((unsigned int) ((dev) & MINORMASK))
>> -#define MKDEV(ma,mi)	(((ma) << MINORBITS) | (mi))
>> +#define MKDEV(ma, mi)	(((ma) << MINORBITS) | ((mi) & MINORMASK))
>>
>>  #define print_dev_t(buffer, dev)					\
>>  	sprintf((buffer), "%u:%u\n", MAJOR(dev), MINOR(dev))
> 
> Shouldn't the users of MKDEV() be fixed instead of changing the MKDEV()
> definition?
> 
> Thanks,
> 
> Bart.
Thanks for your reply.
I think that your opinion is much better. Users of MKDEV() should
make sure that the mi is not larger than MINORMASK. If we mask mi with
MINORMASK in MKDEV(), ma will be not affected by mi. But, the result
may be not the expected value of users.

So, please ignore the patch.


> 
> 
>
diff mbox series

Patch

diff --git a/include/linux/kdev_t.h b/include/linux/kdev_t.h
index 85b5151911cf..40a9423720b2 100644
--- a/include/linux/kdev_t.h
+++ b/include/linux/kdev_t.h
@@ -9,7 +9,7 @@ 

 #define MAJOR(dev)	((unsigned int) ((dev) >> MINORBITS))
 #define MINOR(dev)	((unsigned int) ((dev) & MINORMASK))
-#define MKDEV(ma,mi)	(((ma) << MINORBITS) | (mi))
+#define MKDEV(ma, mi)	(((ma) << MINORBITS) | ((mi) & MINORMASK))

 #define print_dev_t(buffer, dev)					\
 	sprintf((buffer), "%u:%u\n", MAJOR(dev), MINOR(dev))