All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cheah Kok Cheong <thrust73@gmail.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: abbotti@mev.co.uk, hsweeten@visionengravers.com,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] Staging: comedi: comedi_fops: Change comedi_num_legacy_minors type
Date: Wed, 8 Mar 2017 17:38:12 +0800	[thread overview]
Message-ID: <20170308093812.GA2496@linux-Precision-WorkStation-T5500> (raw)
In-Reply-To: <20170307190137.GA6373@kroah.com>

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

  reply	other threads:[~2017-03-08 10:38 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-04 19:22 [PATCH 1/2] Staging: comedi: comedi_fops: Change comedi_num_legacy_minors type Cheah Kok Cheong
2017-03-04 19:22 ` [PATCH 2/2] Staging: comedi: comedi_fops: Fix "out of minor numbers for board device files" Cheah Kok Cheong
2017-03-08  5:54   ` Dan Carpenter
2017-03-08 10:08     ` Cheah Kok Cheong
2017-03-08 11:13       ` Ian Abbott
2017-03-08 16:59         ` Cheah Kok Cheong
2017-03-07 19:01 ` [PATCH 1/2] Staging: comedi: comedi_fops: Change comedi_num_legacy_minors type Greg KH
2017-03-08  9:38   ` Cheah Kok Cheong [this message]
2017-03-08  9:45     ` Greg KH
2017-03-08 10:16       ` Cheah Kok Cheong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170308093812.GA2496@linux-Precision-WorkStation-T5500 \
    --to=thrust73@gmail.com \
    --cc=abbotti@mev.co.uk \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hsweeten@visionengravers.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.