All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Robert T. Johnson" <rtjohnso@eecs.berkeley.edu>
To: Linux Kernel <linux-kernel@vger.kernel.org>
Cc: David Wagner <daw@eecs.berkeley.edu>
Subject: &array considered harmful?
Date: 23 Feb 2004 14:11:49 -0800	[thread overview]
Message-ID: <1077574309.16259.4.camel@dooby.cs.berkeley.edu> (raw)

The kernel has lots of code that takes the address of a local array. 
This works, but it's fragile.  I'd be happy to submit a patch if
everyone agrees that this is a bad programming practice. 

Here's an example of a program that takes the address of an array: 

void func(void) 
{ 
        char A[10]; 
        .... 
        memset(&A, 0, sizeof(A)); 
} 

This works because in C, for a local array, &A == A.  The problem is
that this is very brittle.  If the programmer later decides to allocate
A dynamically, e.g. 

void func(void) 
{ 
        char *A; 
        A = kmalloc(...); 
        .... 
        memset(&A, 0, sizeof(A)); 
} 

then &A is completely different from A, and the code now has a bug. 
Similarly, if the programmer makes A into a parameter, e.g. 

void func(char A[10]) 
{ 
        .... 
        memset(&A, 0, sizeof(A)); 
} 

then A also behaves like a pointer and the code is broken. 

So just about any change to the declaration of A will cause uses of &A
to break.  The good news is that there's no reason to use &A, since just
using "A" will work in all 3 cases: 

void func(void) 
{ 
        char A[10]; 
        .... 
        memset(A, 0, sizeof(A)); 
} 

(Of course, "sizeof(A)" might also break, but that's a separate issue) 

I first noticed this use of &array when using cqual to find user/kernel
pointer bugs in linux.  Since &A has type "pointer to array of char" and
this gets cast to "pointer to void" in the call to memcpy, cqual gets
confused and can generate false positives.  Now, it would be _very_ easy
to change cqual to handle this, but I think it's better to just not use
&A since it breaks easily.  Also, I wonder if this ever comes up in
sparse. 

As I said above, I can generate a patch to eliminate this programming
construct if everyone agrees that it is bad.  What do the kernel
developers think? 

Best, 
Rob 



             reply	other threads:[~2004-02-23 22:19 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-02-23 22:11 Robert T. Johnson [this message]
2004-02-23 22:49 ` &array considered harmful? Mitchell Blank Jr
2004-02-24 12:53 ` Richard B. Johnson

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=1077574309.16259.4.camel@dooby.cs.berkeley.edu \
    --to=rtjohnso@eecs.berkeley.edu \
    --cc=daw@eecs.berkeley.edu \
    --cc=linux-kernel@vger.kernel.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 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.