So I feel a bit bad for saying that moving the pointers around was OK and then saying I didn't like it when you did the work. Sorry for that. The thing is that struct comedi_cmd is sort of the central struct for understanding comedi. It's how user space communicates with the drivers. In do_cmd_ioctl() we should sanitize it and pass it on to the drivers. The drivers should not have to care about the unsanitized version. They should not have to care about how we copied it from user space. This is why I disliked Hartley's original patch where all the drivers had to be modified to use the k_ prefixed version of chanlist. We shouldn't have to explain to the driver writers what the difference is. It should just be that they get sanitized data. Simple. With this new patch we pass on everything except that chanlist is passed as a separate parameter. Why not pass it as we currently do which is the more obvious thing? It means even more that we have to explain the wonky design to the driver authors. I use Sparse regularly. I used to run it on everyone's code, until Fengguang took over. So I think Sparse is important. I hate when people use the same variables to hold data with different endianness. Normally that indicates poor API design. API design is the most important thing. Never write make the code worse for human beings just to please a tool. Sparse helps find bugs and it helps find crap API. But in this case the API is fine. It's the simplest and most obvious way to do things. 1) Take the data from the user. 2) Sanitize the data. 3) Pass the data to the individual drivers. That way everyone is on the same page. I think the existing code makes a lot of sense. Sure we have to add 5 cast statements. But that's a small price to pay. regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
