On Sun, Aug 23, 2020 at 12:57 PM Greg Kroah-Hartman
Re quick fix, I guess it depends on the amount of work for the larger
fix and if we can find volunteers (thanks Himadri!). We need to be
practical as well.
Re:
retval = usb_control_msg(....., data, data_size, ...);
if (retval < buf_size) {
There may be a fine line between interfaces and what code they
provoke. Let me describe my reasoning.
Yes, the current interface allows writing correct code with moderate
amount of effort. Yet we see cases where it's used incorrectly, maybe
people were just a little bit lazy, or maybe they did not understand
how to use it properly (nobody reads the docs, and it's also
reasonable to assume that if you ask for N bytes and the function does
not fail, then you get N bytes).
Currently to write correct code (1) we need a bit of duplication,
which gets worse if data_size is actually some lengthy expression
(X+Y*Z), maybe one will need an additional variable to use it
correctly.
(2) one needs to understand the contract;
(3) may be subject to the following class of bugs (after some copy-paste:
retval = usb_control_msg(....., data, 4, ...);
if (retval < 2) {
This class of bugs won't be necessary immediately caught by kernel
testing systems (can have long life-time).
I would add a "default" function (with shorter name) that does full read:
if (!usb_control_msg(, ...., data, 4))
and a function with longer name to read variable-size data:
n = usb_control_msg_variable_length(, ...., data, sizeof(data)));
The full read should be "the default" (shorter name), because if you
need full read and use the wrong function, it won't be caught by
testing (most likely long-lived bug). Whereas if you use full read for
lengthy variable size data read, this will be immediately caught
during any testing (even manual) -- you ask for 4K, you get fewer
bytes, all your reads fail.
So having "full read" easier to spell will lead to fewer bugs by design.