On Tue, Sep 18, 2012 at 12:39:58PM -0700, Ian Abbott wrote: > On 2012-09-18 20:29, Dan Carpenter wrote: > >I don't like this at all... I feel like you're taking something > >which is so simple and making it so complicated. > > > >This is only used in an unusual way in do_cmd_ioctl() and > >do_cmdtest_ioctl(). It takes literally 5 casts (5 lines of code > >and a few comments) to explain what we are doing. Doing the casts > >actually increases the readability of the code. > > > >The whole point is that we should hide these implementation details > >from the driver. I don't like the k prefix at all. I don't like > >having two huge struct which are virtually identical but except for > >the __user tag. We'd have to keep them in sync and that's a > >nightmare. > > I don't think the overall contents of the structure are going to > change much as it has to be compatible with existing user-space > stuff. So keeping it in sync is mostly a no-op. > > The convention for ioctl interface structures in the kernel's > include/linux/ seems to require the tagging user-space pointers with > __user, so I think comedi's ioctl interface structures should follow > the same convention. 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. regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
