AUDIT: copy_from_user is a deathtrap.
[trimmed cc a bit]
Quoted 10 lines · Denis Vlasenko
Quoted 8 lines · Denis Vlasenko
Quoted 5 lines
Quoted 2 lines
As Linus and others pointed out, copy_{to_from}_user has its uses and will stay, but something like:
I don't say 'kill it', I say 'rename it so that its name tells users what return value to expect'. However, one have to weigh
Why?
Why what? Why rename
copy_to_user? Because in its current form people
Why rename it.
Quoted 3 lines
misunderstand its return value and misuse it.
We can keep unmodified version ofcopy_to_userfor some time for compatibility.
Function name is not documentation. Documentation documents function
API, or, in case documentation is not available, source does it. copy_to_user_dude_I_return_uncopied_length_on_error_not_EFAULT_parameters_are_same_as_for_memcpy() is unacceptable name, and anything shorter does not document things
you must know.
Quoted 8 lines
Quoted 4 lines
From copyin/out descriptions sent yesterday if you want same source code running on all (BSD,SVR4,OSF/1) platforms, you must do
if (
copyin()) return [-]EFAULT;But if I am new to Linux and just want to write my first piece of kernel code,
copyout()is even worse thancopy_to_user():
it too lacks info of what it can return (0/1, 0/-EFAULT, # of copied bytes,
Hey, please change sys_read(). I'm used that read() returns -1 on error, but now in kernel it returns some strange negative value. Please fix it.
I'm not sure that I want to use kernel which contains code written by people who do not read API documentation. I expect that everyone here tests every branch in code he writes at least once - and such test would (f.e.) quickly reveal that read(fd, NULL, 1000) does not return -1 & set errno to EFAULT, but instead returns complete success (1000) on affected sound drivers.
No. copy_*_user interface is documented since 2.1.0, only bad old verify_area() in older kernels returned -EFAULT on error, and I do not believe that coders who coded for 2.0.x did not notice completely different interface (copy_from_user instead of verify_area + memcpy_fromfs) during last almost 6 years.
Best regards,
Petr Vandrovec [email protected]
Quoted 5 lines · Petr Vandrovec
Function name is not documentation. Documentation documents function API, or, in case documentation is not available, source does it.
copy_to_user_dude_I_return_uncopied_length_on_error_not_EFAULT_parameters_are_same_as_for_memcpy()is unacceptable name, and anything shorter does not document things you must know.
Anything can be taken to the ridiculous extreme: "let's use ci() and co()! We will document 'em and everything will be fine!"
Let's abstain from this type of discussion.
Quoted 6 lines
I'm not sure that I want to use kernel which contains code written by people who do not read API documentation. I expect that everyone here tests every branch in code he writes at least once - and such test would (f.e.) quickly reveal that
read(fd, NULL, 1000)does not return -1 & set errno to EFAULT, but instead returns complete success (1000) on affected sound drivers.
In the ideal world. We're in the real world and there *is* broken code, as demonstrated by Rusty. If we can reduce chances of programming errors by choosing good names, we have to do it.
Which name is 'good'/too short/too long is up to "big boys" to decide. I presume they know better, I don't hack on kernel day and night. They do. --
vda
Quoted 2 lines · Denis Vlasenko
Which name is 'good'/too short/too long is up to "big boys" to decide. I presume they know better, I don't hack on kernel day and night. They do.
Actually, in this case I would say it's the occasional hacker who is more likely to need good names, and consequently better placed to suggest them; the big boys presumably know this stuff so well it is a non-issue to them.
That said, I agree with Linus that copy_from_user et al is better than copyin, for reasons expressed earlier.
My work as a technical writer often results in comments like "I didn't understand that" or "no, it's not like that", when in fact the words are correct. I still take such reports seriously, though, as the point of writing English is to convey meaning from you to someone's head, not to the paper.
All I can suggest about naming function calls is that consistency is generally better than point solutions.
Ruth
P.S. Might it be possible to avoid misuse of copy_ return values by changing the return type in some way?
Signature · 2 lines
Ruth Ivimey-Cook Software engineer and technical writer.