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

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

 



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.

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