Re: [PATCH 3/9] staging: comedi: change type of auto-config context

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

 



On 2012-10-22 17:21, H Hartley Sweeten wrote:
On Saturday, October 20, 2012 11:37 AM, Ian Abbott wrote:
On 20/10/12 04:44, Dan Carpenter wrote:
On Mon, Oct 15, 2012 at 01:07:33PM +0100, Ian Abbott wrote:
Change the type of the context parameter passed to
`comedi_auto_config_helper()` from `void *` to `unsigned long`.  It's
currently just an internal change and all current internal usages pass
pointers in the context, but future uses of this function may pass
                                                              ^^^
integer values

"May?"  Don't make this kind of change until you have a user ready.
Using an unsigned long is uglier than using a void pointer so let's
not do it until it is needed.

Obviously the same thing would apply to the other places.

Patch 6 passes an unsigned integer value in the context.  In the end,
the context probably won't be used much, but it's handy to have it in
case it's needed.

Patch 6 has a different issue..

You want to change the context from a void * to an unsigned long.. OK..
In Patch 6 you are passing a context of COMEDI_AUTO_ATTACH_{PCI,USB}.
You define these as:

+/* auto_attach() context values */
+#define COMEDI_AUTO_ATTACH_PCI	(-1UL)	/* from comedi_pci_auto_config() */
+#define COMEDI_AUTO_ATTACH_USB	(-2UL)	/* from comedi_usb_auto_config() */

Seems goofy... -1 and -2 are not 'unsigned'...

I could change them to (~0UL) and (~1UL).

To me it just seems cleaner to leave the context as a void *. If you want
to pass a non-pointer context, cast that context to a void * and then back
to its real type when used. Casting a context that is a pointer to some type
of struct to an unsigned long and then back to the struct seems messy.

Your concern is noted, but I intend to eliminate all the functions where that is currently done, namely comedi_old_auto_config() plus comedi_old_auto_config_wrapper(), comedi_new_pci_auto_config() plus comedi_pci_attach_wrapper(), and comedi_new_usb_auto_config() plus comedi_usb_attach_wrapper().

--
-=( 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/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