views:

122

answers:

6

Hello,

gcc 4.4.3 c89

I have the following source code. And getting a stack dump on the printf.

char **devices;
devices = malloc(10 * sizeof(char*));

strcpy(devices[0], "smxxxx1");

printf("[ %s ]\n", devices[0]); /* Stack dump trying to print */

I am thinking that this should create an char array like this.

devices[0]
devices[1]
devices[2]
devices[4]
etc

And each element I can store my strings.

Many thanks for any suggestions,

== Added correction ===

for(i = 0; i < 10; i++)
{
    devices[i] = malloc(strlen("smxxxx1")+1);
}
+1  A: 

you have allocated space for pointers (devices) but have not allocated space for strings you are going to store.

skwllsp
+2  A: 

You only have allocated an array of pointers to character-arrays. You will have to allocate memory for each string you plan to store:

char **devices;
devices = malloc(10 * sizeof(char*));

//Added this line:

devices[0] = (char*)malloc(strlen("smxxxx1")+1);
strcpy(devices[0], "smxxxx1\0");

printf("[ %s ]\n", devices[0]); /* Stack dump trying to print */
sum1stolemyname
Your strings really don't need to include `\0` explicitly, they already are null terminated
Hasturkun
Just wondering as an extra precaution. Should the actual char array be NULL terminated. I have 10 elements. However, the 10th should be NULL. This is in case you are looping through displaying and you want to stop at the NULL? Just an idea Thanks.
robUK
@robUK: Using NULL as a sentinel value at the end of an array is sometimes common practice (e.g. `argv`). It depends on the situation; sometimes there might not be an appropriate value that can be used as a sentinel (what if NULL were a legitimate element in your array? what if you aren't dealing with an array of pointers?).
jamesdlin
+3  A: 

You have allocated memory for storing 10 char pointers. To store a string at these memory location you have to allocate memory for each of them. Basically you need something like device[0] = malloc(stringLen + 1); for each pointer.

Naveen
+5  A: 

You have allocated memory for an array of pointers. You need to allocate the memory for each element to store the string

e.g.

#define NUM_ELEMENTS 10
char **devices;
devices = malloc(NUM_ELEMENTS  * sizeof(char*));

for ( int i = 0; i < NUM_ELEMENTS; i++)
{
    devices[i] = malloc( length_of string + 1 );
}
Mark
You introduce NUM_ELELENTS and still use 10 in the malloc() call.
sharptooth
+4  A: 

devices[0] is a char *, but you haven't allocated any storage for it. Do this instead:

char **devices;
devices = malloc(10 * sizeof(char*));

devices[0] = strdup("smxxxx1");

printf("[ %s ]\n", devices[0]);

Eventually, you'll have to free the memory allocated by strdup():

free(devices[0]);
Marcelo Cantos
A: 

devices is an array of pointers. You are copying the string "smxxxx1" over the array, when it looks like you want to set element 0 to point to the string.

Instead of the strcpy() try:

devices[0] = "smxxxx1"

or

devices[0] = strdup("smxxxx1")

Edit:

On a 32 bit system, devices[0] is composed of four bytes. These four bytes are being overwritten with the byte values of the first four characters of the string "smxxxx1". In ascii these are 0x73, 0x6D, 0x78, 0x78. Assuming little-endian addressing, you end-up with devices[0] containing a pointer to the address 0x78786D73. This address will almost certainly not be valid within the process. When the call to printf() tries to dereference this invalid pointer the OS triggers a segmentation fault and dumps core.

The problem was that the OP was incorrectly treating the devices variable as a string (array of char) when initialising it. Its actually an array of pointers to char, and printf() is interpreting it as such.

Andy Johnson
I would upvote this if the OP's array was of pointers to const.
Matt Curtis
Constness is not relevant. Its a language concept. The stackdump is triggered by the OS.
Andy Johnson
@andyjohnson: Sure constness is a language concept, but I'd rather the compiler stop me from writing through a pointer to const than get a kernel trap at runtime.
Matt Curtis