> …do you see a problem bootstrapping that on an obscure code base?

No, I don't. It just requires that you can read C and understand it. Any large codebase (in whatever language) is going to require you to understand certain semantics and idioms, and it doesn't make any sense to document those idioms every single place you use them. Big codebases require context and a comment in some random function isn't going to give you enough context unless it was a tome.

Also, did you see the 2 patches linked in the bug report? They significantly changed the code. C is just like that. It's an environment ripe for comment bitrot. And the only thing worse than no comments is incorrect comments.

In the end, the code is what matters. If you can read it, you can understand it.

Yes, I've seen, read and kind of felt what the patch does. If you claim you _understand_ the code then I call you on this.

The only thing worse about something is the preconceived idea that there is indeed only a _single_ thing worse - all the rest being better or equal.

No, worse than missing comments is also _bad code_ which was the case here. And bad attitudes like "my code is prone to bit rot so I won't comment it" or "my code is self-explanatory so I won't comment it" or worse "my code is so special and super optimized and smart that I will not comment it so only super special guys will be able to modify it". And also "I'm a kernel maintainer so I don't have to comment anything"

Excuse me, based on which experience or do you claim that it was bad code? Based on which experience do you claim that there should be more comments? It looks to me that you don't have anything to support what you say, and I'd appreciate you showing me the opposite.

Have you actually ever programmed anything for Linux kernel? What have you programmed that would be relevant and give the weight to your (for me dubious) claims?

I don't claim anything. The report claims "This appears to be a serious bug". If it wasn't, then they would have not suggested a patch and we would have not have this conversation.

Now, I don't have to show you anything. What are you talking about?

Do you dismiss me because I extrapolate developer attitudes from their obtuse/obscure code (dubious/discuss) or because I can't keep a stack of callbacks in my mind to get myself out of the maze (true by the way)?

But what about that function? It has 5 arguments! Do you claim that they need no explanation whatsoever? And don't cheat. Tell me without looking what is "type" suppose to contain?

[1]http://lxr.free-electrons.com/source/net/compat.c#L223

It was bad code because it dereferenced a user mode pointer without going through copy_from_user/copy_to_user. That much is clear.

That's not to say that nobody else has made this mistake or that it won't be made again by otherwise capable programmers. One can talk about this as bad code while still having sympathies for the error. Hate the sin, not the sinner.

The issue I raised is not about this particular piece of code or about it being defective - it could have been perfect for what is worth.

The deeper issue is about code as it is communicated to the next developer. For various reasons (most - me included) developers do not talk to the next developer but to the compiler. And some other guy comes later who doesn't know about copy_to/from_user shit because he is not exposed to the internals.

He might be competent programmer but he lacks exposure. He might also be able to see in a second the dereference but when he is reviewing the code, he keeps a stack of several levels of irrelevant contexts that blurred his view.

He could have avoided all that if only the code was being written with him in mind.

Having done a small amount of work with the Linux kernel I do not consider this to be an informed position. There are a lot of conventions being followed here and they are followed consistently throughout the code base - they become more and more obvious the more code you read. Not knowing about copy_{from,to}_user is a very rookie mistake. I find it hard to believe the author of this code didn't know about it (they even annotated the parameter as a user pointer), my guess is they probably just figured that the functions they were calling already performed this check and didn't think it needed to be inserted twice.

Edit: Re-reading your comment I also wanted to point out that the problem of handling user mode pointers in a kernel is a difficult one, and requires discipline. You can't keep a straight face and say as you do that you can handle it in a way that newbies can come right in and write bug free code without learning these conventions. The only sane way is to create a workable set of conventions, apply them consistently, and do what you can to educate new contributors as they come along. It's not unique to Linux - I used to work at MS and can tell you the NT kernel has the same issues.

Man - don't take my example as something to extrapolate from. See the big picture and where my example comes from. Extrapolate from there.

This particular piece of code is simple and easy to understand and follow. The function is small - you can take a few hours and understand the stack.

The point is, you have to play the compiler game in order to understand. The original coder is not helping you in any way. It doesn't give you any context. Where's the place of this (any) function? Why was it written in the first place? What problem is it trying to solve?

There are some things that the code alone can't tell. Because the code talks to the compiler and the compiler doesn't care about context - people do. And people understand either by taking the hard way (playing the compiler game - having to jump back & forth) or talking to a human. And the code comments are the best that you can get short of talking directly to the developer.

I think you have abstracted yourself too far away from the details here.

I'm all about leaving code maintainable for the next guy, however I think we disagree about what that means. To me, it's largely about writing your code in consistent patterns that make certain classes of bug "pop out" at you (because doing things that way would stand out and look like a break from the convention).

However as a practical concern, if you're writing kernel code you shouldn't assume your future maintainers don't know about the distinction between user and kernel address spaces. And you shouldn't have to be very detailed about every small little compatibility shim you write - if compat_sys_recvfrom does very little else but call sys_recvfrom without much comment or fanfare that is OK by me (keep in mind there's going to be one of these wrappers for almost every syscall).

Indeed, you have to draw a line somewhere. Do you explain (1) 2+2, (2) user/kernel space, (3) what a _compat usually is for or (4) what this particular _compat does?

I for one would draw the line before (4). The original developer drew it after. If I would explain API conventions I would draw it before (3) but crypto _API_ for example has the line after (4). And there are appropriate places for (1) and (2) also.

The point is, you address to some audience. But even if the code is clean, a newcomer will have a hard time getting up to speed if he has no idea about the context. The way I see it, for some parts of the kernel, either the code is too precious to be touched or the maintainers do not have any interest in explaining it to an outsider. It's like they are loosing knowledge by sharing.

Think new comers. How can you make them contribute? Or maybe the barrier to entry is high enough on purpose?