Re: [PATCH 0/1] Constify struct address_space_operations for2.6.32-git-053fe57ac v2

From: Al Viro
Date: Tue Dec 15 2009 - 19:04:36 EST


On Wed, Dec 16, 2009 at 12:28:36AM +0100, Emese Revfy wrote:

> Notice how you got an error for line 12 (p->g assignment) but no warning or error
> at all for line 11 (p->f assignment). This example illustrates what I was explaining
> so far:
>
> if you dynamically allocate an ops structure (the result of which is a pointer type like
> p in the above example) then with a non-const structure field you get no indication
> from the compiler that you are doing something undesired, whereas with a const
> structure field you get an error immediately. This is what helps a future developer
> as it gives him a hint that he is doing something wrong and if he still insists on his
> way of dynamic allocation, he will have to come up with ugly code
> (e.g., void *(**)(void))(&p->g) = 0) that will not pass human review. You can teach gcc,
> sparse, checkpatch, etc to recognize some of this ugliness but you cannot
> programmatically detect all possible ways of evasion.
> And if the compiler can help the developers, why not make use of it?

Sigh... Could you please stop assuming that nobody knows C? Your point
about const members of struct making the entire lvalue non-modifiable
is *NOT* *ARGUED*. It is correct.

HOWEVER, such use of const is highly non-idiomatic. Combined with the
general clumsiness of syntax for qualifiers, it creates a stumbling
block for human readers.

Compiler is not the only thing that has to parse C; programmers need to do
the same and hitting slow paths in our parsers is a Bad Thing(tm).
_The_ requirement for any kind of annotations is non-intrusiveness for
casual human reader. And that's what is violated here.

You are trying to express a property of struct; however, it ends up spread
all over the members' declarators. Bad for readers...
--
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/