Re: [PATCH 1/2] Staging: comedi: comedi_fops: Change comedi_num_legacy_minors type

From: Cheah Kok Cheong
Date: Wed Mar 08 2017 - 05:38:47 EST


Dear Greg,
Thanks for taking the time to review.

On Tue, Mar 07, 2017 at 08:01:38PM +0100, Greg KH wrote:
> On Sun, Mar 05, 2017 at 03:22:32AM +0800, Cheah Kok Cheong wrote:
> > Change to unsigned to allow removal of negative value check in
> > init section.
>
> Why?
>

User can input a -ve number as parameter for module loading.
This will be caught by the mentioned check and cause loading to fail.
I think the original intention here is to inform user via kernel log
the acceptable input range.

Now if a user doesn't know how to access the log, it's of no help.

If a user does know how to access the log, probably also know how
to use modinfo or know that reserving a -ve number of minors is
not acceptable.

IMHO, this check is pointless and best enforced in module_param.

> > Use smaller data type since the max possible value currently is 48.
>
> Does it matter?
>

You are right it doesn't matter and not worth the effort doing unless
concurrent work is being done in this area.
Which is what patch 2/2 is trying to do here.
Please note that I've sent V2. Patch 1/2 unchanged and 2/2 with amended
commit log.

Thanks,
Brgds,
CheahKC

> thanks,
>
> greg k-h