&array considered harmful?

&array considered harmful?

[Robert T. Johnson]

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 

Re: &array considered harmful?

[Mitchell Blank Jr]

Robert T. Johnson wrote:
>         memset(&A, 0, sizeof(A)); 
[...]
> This works because in C, for a local array, &A == A.  The problem is
> that this is very brittle.

I'm probably in the minority here, but I've gotten into the habit of saying
"&A[0]" since I think it's more explicit ("I want the address of the
FIRST ELEMENT of the array") and it avoids exactly the problems you mention.
It's true that it's equivelent to just saying "A" (well, almost - if "A"
is a pointer then it could be an lvalue while "&A[0]" never is) but I
like the visual cue of that "&" provides.  Matter of taste I guess -
I'm sure some people consider it ugly.

-Mitch

Re: &array considered harmful?

[Richard B. Johnson]

On Mon, 23 Feb 2004, Robert T. Johnson wrote:

> 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));
> }
>

[SNIPPED...]
You are preaching to the choir when it comes to code like that.
However, even lint allows it! I first thought it was a GNUism
just like void-pointer math being allowed. But, when Lint says
it's okay, I don't think there is any technical reason for
not allowing it because Lint is the most pedantic of pedantica.

Script started on Tue Feb 24 07:46:23 2004
# cat xxx.c
#include <stdio.h>
int main(void);
int main()
{
    char foo[0x10];
    printf("%p\n", foo);
    printf("%p\n", &foo[0]);
    printf("%p\n", &foo);
    return 0;
}
# gcc -Wall --pedantic -o xxx xxx.c
# lint xxx.c
LCLint 2.2a --- 04 Sep 96
Finished LCLint checking --- no code errors found
# exit
exit
Script done on Tue Feb 24 07:47:03 2004

Cheers,
Dick Johnson
Penguin : Linux version 2.4.24 on an i686 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.