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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
