On Thu, 15 Nov 2007, Michael Gerdau wrote:

quoted text
> > This code is far to be perfect, some part is outdated, bcopy() use instead
> > of memcpy() for example. More annoying are the comment, the file is 3306
> > lines while there is only 1640 line of code, nothing bad per se but looking
> > some comments:
> > 
> >  		/*
> >  		 * Before we begin this operation, disable kernel preemption.
> >  		 */
> >  		kpreempt_disable();
> 
> <disclaimer>
> I'm not a kernel developer.
> </disclaimer>
> 
> That having said:
> I really do like such obvious (as in: for those knowing the stuff anyway)
> comments when looking at code and probably concepts I'm not familiar with.
>
> ...
> 
> I mean, isn't the whole purpose of comments to help those not familiar
> with the code to understand it's purpose and possibly the intention of
> the author (just in case the author had coded a bug) ?

That's the problem with really obvious comments. In the example above, 
that function had better disable kernel preemption with a name like that, 
and, assuming it's before the code begins the operation in sequence, we 
know when we're doing it. But the comment fails to explain why we need to 
disable kernel preemption before beginning the operation, just that we are 
doing so. Having the comment merely distracts the reader from the fact 
that the purpose of the code and the intention of the author are 
completely undocumented. And there's a realy chance that this comment or 
ones like it cause this statement and the place in the code where things 
would go wrong if preemption weren't disabled to not fit on the reader's 
screen together, so it is not only unclear what the author's intention 
was, but it is harder to figure out from looking at the code than it would 
be without comments, because fewer clues are actually visible at the same 
time, since each of them takes up extra screen space.

The code itself should be written to tell the reader everything there is 
to know about what it does, and the comments in code should only tell the 
reader why it does that.

	-Daniel
*This .sig left intentionally blank*
-
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/