[PATCH 01/50] staging: comedi: prepare for new struct comedi_kcmd

H Hartley Sweeten hartleys at visionengravers.com
Wed Sep 19 06:32:41 UTC 2012


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


More information about the devel mailing list