From: Greg KH <gregkh@linuxfoundation.org>
To: H Hartley Sweeten <hartleys@visionengravers.com>
Cc: Linux Kernel <linux-kernel@vger.kernel.org>,
devel@driverdev.osuosl.org, fmhess@users.sourceforge.net,
abbotti@mev.co.uk
Subject: Re: [PATCH] staging: comedi: cleanup alloc_subdevices
Date: Mon, 11 Jun 2012 16:56:57 -0700 [thread overview]
Message-ID: <20120611235657.GA26721@kroah.com> (raw)
In-Reply-To: <201206071508.15674.hartleys@visionengravers.com>
On Thu, Jun 07, 2012 at 03:08:15PM -0700, H Hartley Sweeten wrote:
> Cleanup the comedi core "alloc_subdevices" fuction and it's use
> in the comedi drivers.
>
> 1) Move the inline alloc_subdevices() function from comedidev.h
> to drivers.c and rename it to comedi_alloc_subdevices(). The
> function is large enough to warrant being an exported symbol
> rather than being an inline in every driver.
>
> 2) Since dev->n_subdevices is an int variable, change the
> num_subdevices parameter from an unsigned int to an int.
>
> 3) It's possible for a couple of the comedi drivers to incorrectly
> call this function with num_subdevices = 0 so add a sanity check
> before doing the kcalloc.
>
> 4) It's possible for the kcalloc to fail so don't set dev->n_subdevices
> until after the kcalloc has succedded. Also, remove the places in
> the drivers were dev->n_subdevices was being set directly.
>
> 5) Remove all the "allocation failed" error messages.
>
> 6) Remove all the "Allocate the subdevice structures" comments from
> the drivers. The function name itself provides this information.
>
> 7) When comedi_alloc_subdevices does fail, make all the drivers
> properly return the error code.
You are doing 7 different things in one patch. Doesn't that imply that
you should have 7 different patches, in series, doing this? That would
make it easier to review at the least.
Please redo this in that manner and resend.
thanks,
greg k-h
next prev parent reply other threads:[~2012-06-11 23:57 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-07 22:08 [PATCH] staging: comedi: cleanup alloc_subdevices H Hartley Sweeten
2012-06-11 23:56 ` Greg KH [this message]
2012-06-12 0:07 ` H Hartley Sweeten
2012-06-12 0:44 ` Greg KH
2012-06-12 0:49 ` H Hartley Sweeten
2012-06-12 1:42 ` Greg KH
2012-06-12 2:41 ` Arnaud Lacombe
2012-06-12 18:31 ` H Hartley Sweeten
2012-06-12 18:45 ` Greg KH
2012-06-12 19:02 ` H Hartley Sweeten
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=20120611235657.GA26721@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=abbotti@mev.co.uk \
--cc=devel@driverdev.osuosl.org \
--cc=fmhess@users.sourceforge.net \
--cc=hartleys@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.