All of lore.kernel.org
 help / color / mirror / Atom feed
From: Emese Revfy <re.emese@gmail.com>
To: Pavel Machek <pavel@ucw.cz>
Cc: 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,
	viro@zeniv.linux.org.uk, 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 23:24:18 +0100	[thread overview]
Message-ID: <4B295E12.1000408@gmail.com> (raw)
In-Reply-To: <20091216080653.GU24406@elf.ucw.cz>

Pavel Machek wrote:
> Hi!
> 
>>> One const in structure declaration seems to be just enough, see:
>>>
>>> const struct a {
>>> 	void (* f)(void);
>>> 	void (* const g)(void);
>>> } s;
>>>
>>> void h(void)
>>> {
>>> 	struct a *p = &s;
>>> 	s.f = 0;
>>> 	s.g = 0;
>>> 	p->f = 0;
>>> 	p->g = 0;
>>> }
>>>
>>>
>>> delme.c: In function 'h':
>>> delme.c:8: warning: initialization discards qualifiers from pointer target type
>>> delme.c:9: error: assignment of read-only variable 's'
>>> delme.c:10: error: assignment of read-only variable 's'
>>> delme.c:12: error: assignment of read-only member 'g'
>>>
>>> You get clean-enough warnings.
>> 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:
> 
> And notice how you get warning for line 8? That's what I'm talking
> about, and it should be enough to make the developer think about what
> he's doing. 
> 									Pavel

You are talking about in-kernel ops structures whose constness will
prevent bad code from being written. On the other hand, I was talking
about new code yet-to-enter the kernel where the developer has no indication
that he should be using a const ops structure (other than perhaps checkpatch
except apparently things easily fall through the cracks, see my series for
file_operations) and this is where const structure fields would help.

In any case, as I indicated already, I will remove these parts from the patches.
--
Emese

  reply	other threads:[~2009-12-16 22:22 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 08/22] " re.emese
2009-12-13 23:59 ` [PATCH 2/3] Constify struct acpi_dock_ops " re.emese
2009-12-13 23:59 ` [PATCH 09/22] Constify struct backlight_ops " 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
2009-12-16  8:06                       ` Pavel Machek
2009-12-16 22:24                         ` Emese Revfy [this message]
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=4B295E12.1000408@gmail.com \
    --to=re.emese@gmail.com \
    --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=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.