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 Tue, Sep 18, 2012 at 10:29:02PM +0300, 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.
> 
> Really, it's not the right way to go.

Ian, I'm with Dan here, I don't like this, it shouldn't be needed (hint,
almost no other subsystem has something like this, and I'd argue that
any one that does, should be fixed.)

No driver should need to care about this type of thing.  You say in this
thread that you have a way to clean this up, but it needs a "big" patch.
What do you mean by this?  Care to show it with a tiny patch (i.e.
change the core and only one driver), so we can see what you are
thinking of doing?

thanks,

greg k-h
_______________________________________________
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