[PATCH 8/8] staging: comedi: cleanup comedi_alloc_subdevices

[PATCH 8/8] staging: comedi: cleanup comedi_alloc_subdevices

[H Hartley Sweeten]

>From 65db79e570481b04bfc13663fd40de18560f0aa4 Mon Sep 17 00:00:00 2001
From: H Hartley Sweeten <hsweeten@visionengravers.com>
Date: Tue, 12 Jun 2012 11:37:09 -0700
Subject: [PATCH 8/8] staging: comedi: cleanup comedi_alloc_subdevices

Access the individual comedi_subdevices using a pointer instead
of directly accessing as an array. This is how the rest of the
comedi core accesses them.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Ian Abbott <abbott@mev.co.uk>
Cc: Frank Mori Hess <fmhess@users.sourceforge.net>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/staging/comedi/drivers.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 238c29d..c41ddb3 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -58,21 +58,24 @@ struct comedi_driver *comedi_drivers;
 
 int comedi_alloc_subdevices(struct comedi_device *dev, int num_subdevices)
 {
+	struct comedi_subdevice *s;
 	int i;
 
 	if (num_subdevices < 1)
 		return -EINVAL;
-	dev->subdevices =
-	    kcalloc(num_subdevices, sizeof(struct comedi_subdevice),
-		    GFP_KERNEL);
-	if (!dev->subdevices)
+
+	s = kcalloc(num_subdevices, sizeof(*s), GFP_KERNEL);
+	if (!s)
 		return -ENOMEM;
+	dev->subdevices = s;
 	dev->n_subdevices = num_subdevices;
+
 	for (i = 0; i < num_subdevices; ++i) {
-		dev->subdevices[i].device = dev;
-		dev->subdevices[i].async_dma_dir = DMA_NONE;
-		spin_lock_init(&dev->subdevices[i].spin_lock);
-		dev->subdevices[i].minor = -1;
+		s = dev->subdevices + i;
+		s->device = dev;
+		s->async_dma_dir = DMA_NONE;
+		spin_lock_init(&s->spin_lock);
+		s->minor = -1;
 	}
 	return 0;
 }
-- 
1.7.7

Re: [PATCH 8/8] staging: comedi: cleanup comedi_alloc_subdevices

[Dan Carpenter]

On Tue, Jun 12, 2012 at 11:59:55AM -0700, H Hartley Sweeten wrote:
>  	for (i = 0; i < num_subdevices; ++i) {
> -		dev->subdevices[i].device = dev;
> -		dev->subdevices[i].async_dma_dir = DMA_NONE;
> -		spin_lock_init(&dev->subdevices[i].spin_lock);
> -		dev->subdevices[i].minor = -1;
> +		s = dev->subdevices + i;

You don't have to resend, but I think this would look better as:

		s = &dev->subdevices[i];

> +		s->device = dev;
> +		s->async_dma_dir = DMA_NONE;
> +		spin_lock_init(&s->spin_lock);
> +		s->minor = -1;
>  	}

Btw, this patchset is great.  Nice.

regards,
dan carpenter

RE: [PATCH 8/8] staging: comedi: cleanup comedi_alloc_subdevices

[H Hartley Sweeten]

On Tuesday, June 12, 2012 12:50 PM, Dan Carpenter wrote:
> On Tue, Jun 12, 2012 at 11:59:55AM -0700, H Hartley Sweeten wrote:
>>  	for (i = 0; i < num_subdevices; ++i) {
>> -		dev->subdevices[i].device = dev;
>> -		dev->subdevices[i].async_dma_dir = DMA_NONE;
>> -		spin_lock_init(&dev->subdevices[i].spin_lock);
>> -		dev->subdevices[i].minor = -1;
>> +		s = dev->subdevices + i;
>
> You don't have to resend, but I think this would look better as:
>
> 		s = &dev->subdevices[i];

I don't disagree but the "dev->subdevices +i" format is consistently
used in all the comedi stuff. If the format above is preferred we
should probably update everything,

>> +		s->device = dev;
>> +		s->async_dma_dir = DMA_NONE;
>> +		spin_lock_init(&s->spin_lock);
>> +		s->minor = -1;
>>  	}
>
> Btw, this patchset is great.  Nice.

Thanks!
Hartley

Re: [PATCH 8/8] staging: comedi: cleanup comedi_alloc_subdevices

[Dan Carpenter]

On Tue, Jun 12, 2012 at 03:20:32PM -0500, H Hartley Sweeten wrote:
> On Tuesday, June 12, 2012 12:50 PM, Dan Carpenter wrote:
> > On Tue, Jun 12, 2012 at 11:59:55AM -0700, H Hartley Sweeten wrote:
> >>  	for (i = 0; i < num_subdevices; ++i) {
> >> -		dev->subdevices[i].device = dev;
> >> -		dev->subdevices[i].async_dma_dir = DMA_NONE;
> >> -		spin_lock_init(&dev->subdevices[i].spin_lock);
> >> -		dev->subdevices[i].minor = -1;
> >> +		s = dev->subdevices + i;
> >
> > You don't have to resend, but I think this would look better as:
> >
> > 		s = &dev->subdevices[i];
> 
> I don't disagree but the "dev->subdevices +i" format is consistently
> used in all the comedi stuff. If the format above is preferred we
> should probably update everything,

To me the one is way more clear than the other.  If we wanted we
could write any array in terms of pointer math, but arrays are
easier to understand.

regards,
dan carpenter