[PATCH] staging: comedi: have comedi_set_spriv() allocate the memory

[Ian Abbott]

[Originally posted 2013-06-21.  Reposting to new driverdev-devel list.]

On 2013-06-20 18:07, H Hartley Sweeten wrote:
> On Thursday, June 20, 2013 2:50 AM, Ian Abbott wrote:
>> On 2013/06/19 11:24 PM, H Hartley Sweeten wrote:
>>> As suggested by Ian Abbott, comedi_set_spriv() can only be used to
>>> set the subdevice->private pointer to something that can be kfree()'d.
>>> Rename the function to comedi_alloc_spriv() and have it kzalloc() the
>>> memory as well as set the private pointer. This saves a function call
>>> in the drivers and avoids the possibility of incorrectly calling
>>> comedi_set_spriv() for some pointer that is not meant to be kfree()'d.
>>>
>>> Signed-off-by: H Hartley Sweeten <hsweeten at visionengravers.com>
>>> Cc: Ian Abbott <abbotti at mev.co.uk>
>>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>>
>> Looks good.
>>
>> Reviewed-by: Ian Abbott <abbotti at mev.co.uk>
>
> Thanks.
>
> Question.
>
> In comedi_alloc_spriv() do you think the spinlock of the
> runflags is necessary? This function should only be called
> during the attach of a board so I feel setting SDF_FREE_SPRIV
> should be safe without the spinlock.

It should be fine without the spinlock, as you say.

> I would like to move this function from comedi_fops.c to
> drivers.c but I don't want to unnecessarily expose the
> comedi_set_subdevice_runflags() helper since only the
> core should be messing with the runflags.

Does it really matter if it's in comedi_fops.c or drivers.c?  Or are you 
planning to move other some other comedi driver core functions into 
drivers.c as well?

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-