On Tuesday, September 18, 2012 9:57 PM, Ian Abbott wrote: > On 18/09/12 20:48, Dan Carpenter wrote: >> Adding __user annotations is one way of avoiding bugs, so it's >> better to put them throughout, yes. But if you look at how badly it >> mangles the code then it's not worth it at all. Complicated, messy >> code causes more bugs than anything else. > > I have an alternate suggestion for discussion which keeps the `_user` > tags, avoids the casting, and avoids shadowing the `struct comedi_cmd`. > The suggestion is to avoid using the `chanlist` in this structure for > point to the kernel copy of the channel list in the same way that we > currently avoid using the `data` pointer in `struct comedi_insn` to > point to the kernel copy of the instruction data. Ah... So that's why so of the subdevice ops have the 'unsigned int *data' parameter. It's actually a kernel copy of the user insn->data. Cool. Now I can fix the sparse issues in the s526 driver. :-) > This change would involve: > > (a) adding a `chanlist` pointer to `struct comedi_async`; > (b) modifying all the low-level `do_cmd()` handlers to use > `async->chanlist` instead of `async->cmd.chanlist`; > (c) adding a `chanlist` parameter to the low-level `do_cmdtest()` > handlers (and changing them to use it instead of `cmd->chanlist`). > > This would have to be done as one or two massive patches to avoid > intermediate breakage. I think this approach sounds good. I'll take a look at it tommorow (unless you beat me to it). Regards, Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel