Re: [PATCH v3] staging: comedi: drivers: let core handle freeing s->private

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

On 2013-06-11 19:32, H Hartley Sweeten wrote:
Introduce a new subdevice runflags, SRF_FREE_SPRIV, and a new helper
function, comedi_set_spriv(), that the drivers can use to set the
comedi_subdevice private data pointer. The helper function will also
set SRF_FREE_SPRIV to allow the comedi core to automatically free the
subdevice private data during the cleanup_device() stage of the detach.

Currently s->private is only allocated by the 8255, addi_watchdog,
amplc_dio200_common, and ni_65xx drivers. All users of those drivers
can then have the comedi_spriv_free() calls removed and in many cases
the (*detach) can then simply be the appropriate comedi core provided
function.

The ni_65xx driver uses a helper function, ni_65xx_alloc_subdevice_private(),
to allocate the private data. Refactor the function to return an errno
or call comedi_set_spriv() instead of returning a pointer to the private
data and requiring the caller to handle it.

Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
v3: * move the new flag from the subdevice 'subdev_flags' in comedi.h to
       the 'runflags' in comedidev.h so the implementation detail is hidden
       from the user
     * add comedi_set_spriv()
     * refactor the subdevice private data allocation in the ni_65xx driver
v2: * missed removing a couple unused variables

Reviewed-by: Ian Abbott <abbotti@xxxxxxxxx>

It's okay as is, but I've thought of a possible improvement to make it a bit cleaner:

Since comedi_set_spriv() can only be used to set the subdevice->private pointer to something that can be kfree()'d, why not have a comedi_alloc_spriv() function that combines your comedi_set_spriv() with a kzalloc()? This saves a function call and avoids the possibility of some incorrect code calling comedi_set_spriv() for some pointer that is not meant to be kfree()'d.

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux