[Ffmpeg-devel] Re: [xine-devel] Suspicious code in xine-lib CVS from 2006-04-16 18:43

[Morten Nilsen]

M?ns Rullg?rd wrote:
> Gary Corcoran <gcorcoran at rcn.com> writes:
> 
>> M?ns Rullg?rd wrote:
>>> Some people (presumably those who have difficulties understanding C
>>> code) insist that a comment mentioning the absence of a break
>>> statement just before a case label.  These are the same kind of people
>>> that put comments like /* add 1 to i */ next to an i++ statement.
>> I disagree.  A /*FALLTHROUGH*/ (or similar) comment, where one would
>> normally expect a break statement, lets a reviewer of the code _know_
>> that that is the intent of the writer.  Otherwise he might just have
>> forgotten to add a break; (e.g. as a result of a bad copy-paste operation).
>> That is, it could well be unintended and thus a bug.
> 
> A competent person will be able see what the intent was without
> relying on comments.

I agree :) that makes it two to one..

as code is (should be) tested before commit, the code obviously works as 
it is, and hence, it is correct.. also, if you know what the code is 
supposed to do, fallthrough will make sense or be a blatant mistake..

And, if you don't understand what the code is supposed to do, you have 
no business reviewing the code.

-- 
Morten
:wq