Re: [PATCH] fs/fcntl.c - remove impossible <0 check in do_fcntl -arg is unsigned.

[Paul Jackson]

Jesper wrote:
> I can't argue with that, but I don't think this patch actually decreases
> readabillity. It's still perfectly clear what the remaining code does, and
> if anybody is wondering if 'arg' could ever be <0 then a quick glance at
> the type will answer that.

The basic thought likely to go through the head of someone first
thinking about this bit of logic is that arg has to be between 0 and
_NSIG to be valid.  It then requires a second thought to realize that
since arg is 0, you don't have to check explicitly for arg < 0.

Everytime we can avoid requiring 'a second thought', we (feable minded
humans) win.


> Would you like this sort of patch better if removing the code went
> hand-in-hand with the addition of a one-line comment stating something
> like  /* the test for arg < 0 is not done since arg is unsigned */ or ?

No - such a comment doesn't remove the 'second thought' in this case.
Rather, it emphasizes it.

It is the sort of comment that (1) I find myself writing often, and then
(2) later replacing with code that renders the comment irrelevant.

It's one of those little "reader beware" (caveat lector) comments that
often mark places where a little code refinement can remove a comment.
Whenever a comment that explains why something is or is not done or is
special cased can be replaced with code that just does (or doesn't do)
it or doesn't need a special case, with no loss of form, fit, function
or performance, that's likely a win-win.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@xxxxxxx> 1.650.933.1373
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/