public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Emese Revfy <re.emese@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>,
	Arjan van de Ven <arjan@infradead.org>,
	Paul Mundt <lethal@linux-sh.org>, Matthew Wilcox <matthew@wil.cx>,
	linux-kernel@vger.kernel.org, torvalds@linux-foundation.org,
	akpm@linux-foundation.org
Subject: Re: [PATCH 0/1] Constify struct address_space_operations for 2.6.32-git-053fe57ac v2
Date: Wed, 16 Dec 2009 00:04:22 +0000	[thread overview]
Message-ID: <20091216000422.GY14381@ZenIV.linux.org.uk> (raw)
In-Reply-To: <4B281BA4.8090806@gmail.com>

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...

  reply	other threads:[~2009-12-16  0:04 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-13 23:58 [PATCH 00/22] Constify struct backlight_ops for 2.6.32-git-053fe57ac v2 re.emese
2009-12-13 23:58 ` [PATCH 01/22] " re.emese
2009-12-13 23:58 ` [PATCH 02/22] " re.emese
2009-12-13 23:58 ` [PATCH 03/22] " re.emese
2009-12-13 23:58 ` [PATCH 04/22] " re.emese
2009-12-13 23:58 ` [PATCH 05/22] " re.emese
2009-12-15 22:47   ` Richard Purdie
2009-12-16 22:39     ` Emese Revfy
2009-12-13 23:58 ` [PATCH 06/22] " re.emese
2009-12-13 23:58 ` [PATCH 1/3] Constify struct acpi_dock_ops " re.emese
2009-12-13 23:58 ` [PATCH 07/22] Constify struct backlight_ops " re.emese
2009-12-13 23:59 ` [PATCH 2/3] Constify struct acpi_dock_ops " re.emese
2009-12-13 23:59 ` [PATCH 08/22] Constify struct backlight_ops " re.emese
2009-12-13 23:59 ` [PATCH 09/22] " re.emese
2009-12-13 23:59 ` [PATCH 3/3] Constify struct acpi_dock_ops " re.emese
2009-12-13 23:59 ` [PATCH 10/22] Constify struct backlight_ops " re.emese
2009-12-14  0:27   ` Jonathan Woithe
2009-12-13 23:59 ` [PATCH 11/22] " re.emese
2009-12-13 23:59 ` [PATCH 12/22] " re.emese
2009-12-13 23:59 ` [PATCH 13/22] " re.emese
2009-12-13 23:59 ` [PATCH 14/22] " re.emese
2009-12-13 23:59 ` [PATCH 15/22] " re.emese
2009-12-13 23:59 ` [PATCH 16/22] " re.emese
2009-12-13 23:59 ` [PATCH 1/1] Constify struct address_space_operations " re.emese
2009-12-13 23:59 ` [PATCH 17/22] Constify struct backlight_ops " re.emese
2009-12-13 23:59 ` [PATCH 18/22] " re.emese
2009-12-13 23:59 ` [PATCH 19/22] " re.emese
2009-12-13 23:59 ` [PATCH 20/22] " re.emese
2009-12-13 23:59 ` [PATCH 21/22] " re.emese
2009-12-13 23:59 ` [PATCH 22/22] " re.emese
2009-12-14  0:38 ` [PATCH 0/1] Constify struct address_space_operations " Matthew Wilcox
2009-12-14  1:33   ` Emese Revfy
2009-12-14  2:19     ` Paul Mundt
2009-12-14  7:08       ` Emese Revfy
2009-12-14 11:26         ` Pavel Machek
2009-12-14 16:00           ` Arjan van de Ven
2009-12-14 16:30             ` Matthew Wilcox
2009-12-14 21:25             ` Pavel Machek
2009-12-14 22:17               ` Arjan van de Ven
2009-12-14 22:21                 ` Pavel Machek
2009-12-14 22:41                 ` Emese Revfy
2009-12-15 18:14                   ` Pavel Machek
2009-12-15 23:28                     ` Emese Revfy
2009-12-16  0:04                       ` Al Viro [this message]
2009-12-16  8:06                       ` Pavel Machek
2009-12-16 22:24                         ` Emese Revfy
2009-12-14 23:13             ` Emese Revfy
2009-12-15 10:47               ` Pavel Machek
2009-12-15 19:12             ` Al Viro
2009-12-14 12:36         ` Paul Mundt
2009-12-14 22:20           ` Emese Revfy
2009-12-15  0:01             ` Arjan van de Ven
2009-12-15 23:53               ` Emese Revfy
2009-12-14 11:18     ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091216000422.GY14381@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=arjan@infradead.org \
    --cc=lethal@linux-sh.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew@wil.cx \
    --cc=pavel@ucw.cz \
    --cc=re.emese@gmail.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox