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'...
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.
Just my opinion...
Regards,
Hartley
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
