[PATCH 00/37] staging: comedi: some comedi core reorganization

Greg Kroah-Hartman gregkh at linuxfoundation.org
Fri Apr 5 21:30:37 UTC 2013


On Fri, Apr 05, 2013 at 03:34:22PM +0100, Ian Abbott wrote:
> On 2013/04/04 09:21 PM, H Hartley Sweeten wrote:
> >A comment about the reference counts issue might be a good idea
> >so it isn't forgotten.
> 
> In brief, my plan is to add a struct kref to the struct
> comedi_device, and point comedi_class->dev_release to a function
> that does a kref_put() on that kref.  We need to do a kref_get() for
> each of the class devices created for the subdevices, but not for
> the class device created for the main comedi device.  The
> kref_get()s for the subdevices will be balanced out by the
> ->dev_release()/kref_put() calls when those subdevice class devices
> are destroyed.  The extra ->dev_release/kref_put() resulting from
> destruction of the main class device will result in the struct
> comedi_device being freed.
> 
> >Question. Are all the BUG_ON checks of the minor actually needed?
> >comedi_alloc_subdevice() and comedi_alloc_subdevice_minor() both
> >validate the minor before setting s->minor. The BUG_ON checks in
> >comedi_free_subdevice_minor() should never happen and the others
> >shouldn't be possible either.
> 
> Good question.  Of course bugs should never happen. ;)  How much
> checking for potential bugs is done is somewhat subjective.  Should
> the code assume nothing but the core will change s->minor or play it
> safe and check for a potential bug?

Check, and error out, but never call BUG_ON() from a driver, as you just
crashed someone's box.  WARN_ON() would be best.

I'll leave this for now, but in the future it should be changed.

thanks,

greg k-h



More information about the devel mailing list