linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pekka Enberg <penberg@cs.helsinki.fi>
To: Linux Kernel List <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@osdl.org>,
	Al Viro <viro@ftp.linux.org.uk>, Andrew Morton <akpm@osdl.org>
Subject: Re: p = kmalloc(sizeof(*p), )
Date: Tue, 20 Sep 2005 14:18:42 +0300	[thread overview]
Message-ID: <84144f0205092004187f86840c@mail.gmail.com> (raw)
In-Reply-To: <20050918100627.GA16007@flint.arm.linux.org.uk>

Hi,

On 9/18/05, Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> 1. The above implies that the common case is that we are changing the
>    names of structures more frequently than we change the contents of
>    structures.  Reality is that we change the contents of structures
>    more often than the names of those structures.
> 
>    Why is this relevant?  If you change the contents of structures,
>    they need checking for initialisation.  How do you find all the
>    locations that need initialisation checked?  Via grep.  The problem
>    is that:
> 
>         p = kmalloc(sizeof(*p), ...)
> 
>    is not grep-friendly, and can not be used to identify potential
>    initialisation sites.  However:
> 
>         p = kmalloc(sizeof(struct foo), ...)
> 
>    is grep-friendly, and will lead you to inspect each place where
>    such a structure is allocated for correct initialisation.

I would disagree on this one. You can still grep all the places where
the local variable is declared in. Furthermore, structs are not always
initialized where they're kmalloc'd so you need to manually inspect
anyway.

On 9/18/05, Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> 2. in the rare case that you're changing the name of a structure, you're
>    grepping the source for all instances for struct old_name, or doing
>    a search and replace for struct old_name.  You will find all instances
>    of struct old_name by this method and the bug alluded to will not
>    happen.

Perhaps it has poor wording but I was more thinking about a case where
you shuffle code around and forget that you changed a struct to
something else (not necessarily removing the old one).

On 9/18/05, Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> So the assertion above that kmalloc(sizeof(*p) is somehow superiour is
> rather flawed, and as such should not be in the Coding Style document.

I think it is better because:

  - It is easier to get right.
  - It is easier to audit with a script.
  - It is shorter.

I am not saying you can use sizeof(*p) everywhere but it is the common
case and as such the preferred form.

                            Pekka

  parent reply	other threads:[~2005-09-20 11:18 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-09-18 10:06 p = kmalloc(sizeof(*p), ) Russell King
2005-09-18 11:04 ` Alan Cox
2005-09-18 14:39   ` Al Viro
2005-09-18 16:25     ` Denis Vlasenko
2005-09-18 17:30       ` Al Viro
2005-09-18 18:00         ` Willy Tarreau
2005-09-18 17:47       ` Alan Cox
2005-09-18 16:32 ` Robert Love
2005-09-18 16:52   ` Willy Tarreau
2005-09-18 17:18     ` Al Viro
2005-09-18 17:31       ` Linus Torvalds
2005-09-18 17:45         ` Al Viro
2005-09-18 20:34           ` Roman Zippel
2005-09-18 21:12             ` Al Viro
2005-09-18 21:52               ` Al Viro
2005-09-18 22:25                 ` Linus Torvalds
2005-09-18 23:07                   ` Al Viro
2005-09-20  6:31                     ` Richard Henderson
2005-09-19 21:20                   ` Matthias Urlichs
2005-09-19 21:28                     ` Matthias Urlichs
2005-09-18 19:07         ` Al Viro
2005-09-18 21:30           ` Alan Cox
2005-09-18 21:14             ` Al Viro
2005-09-19  6:09             ` Coywolf Qi Hunt
2005-09-21  2:18         ` Miles Bader
2005-09-18 17:32   ` Randy.Dunlap
2005-09-19  6:47   ` Coywolf Qi Hunt
2005-09-20  8:53   ` Pekka Enberg
2005-09-20  9:39     ` Al Viro
2005-09-20  9:47       ` Pekka J Enberg
2005-09-20  9:53         ` Al Viro
2005-09-20 10:07           ` Pekka J Enberg
2005-09-20 15:14         ` Randy.Dunlap
2005-09-20 11:18 ` Pekka Enberg [this message]
2005-09-20 11:40   ` Russell King
2005-09-20 11:56     ` Denis Vlasenko
2005-09-20 12:20     ` Pekka J Enberg
2005-09-20 12:31       ` Russell King
2005-09-20 12:35         ` Pekka J Enberg
2005-09-20 15:21           ` Randy.Dunlap
2005-09-20 12:53         ` Pekka J Enberg
2005-09-20 17:11         ` Andrew Morton
2005-09-20 17:17           ` Russell King
2005-09-20 18:02           ` Alan Cox
2005-09-20 17:59             ` Andrew Morton
2005-09-20 18:11               ` Russell King
2005-09-20 18:41                 ` Jeff Garzik
2005-09-20 20:41               ` Alan Cox
2005-09-20 19:41             ` Horst von Brand

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=84144f0205092004187f86840c@mail.gmail.com \
    --to=penberg@cs.helsinki.fi \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    --cc=viro@ftp.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).