Array members overwritten with last element

Question

With the following piece of code, I get a very wierd result. Why is the last element's value overwriting all previous array elements? i suspect there's a bigger problem than just this immmediate problem.

#include <stdio.h>

main()
{
    int i, cases;
    char num[1000000];

    scanf("%d", &cases);
    char* array[cases];

    //store inputs in array
    for(i=0; i<cases; i++)
    {
        scanf("%s", &num);
        array[i] = &num;
    }

    //print out array items and their memory addresses
    for(i=0; i<cases; i++)
    {
        printf("%d %s\n", i, array[i]);  //print (array index) (array value) 
        printf("%d %p\n", i, &array[i]); //print (array index) (array address) 
    }
}

Inputs:
3 <-- number of lines to follow
0   <-- put in array[0]
1   <-- put in array[1]
2   <-- put in array[2]

Outputs
0 3         <-- why is this being overwritten with the last element?
0 0013BCD0
1 3         <-- why is this being overwritten with the last element?
1 0013BCD4
2 3
2 0013BCD8

Answer 1

Inside your first loop, you should be (but you aren't) writing each input into a different element of the num array; instead you're always writing to the same place, i.e. to &num.

Answer 2

The upshot here is the line array[i] = # you're setting the value of the array[i] element to the address of the num array; since array is a char array, I suspect it's truncating your num array address, and the low-order byte just happens to be a 3.

However. That said, your char num[1000000] is hideous form, and you should not do that, at all. Allocate on the heap, and choose a smaller number, for heaven's sake. Also, the scanf("%s", &num) won't actually give you what you want. Here's a hint; use a getc() loop to read the numbers; this avoids needing to do any preallocation of an array for scanf().

Answer 3

char* array[cases];

This is allocated at compile-time, not run-time. And cases isn't initialized (although I think you want it to work dynamically anyway.) So you either need to preallocate the memory, or get familiar with the malloc family of library functions.

Answer 4

Thats your code that is fixed:

#include <stdio.h>

main(void)
{
    int i, cases;

    scanf("%d", &cases);
    char* array[cases];

    //store inputs in array
    for(i=0; i<cases; i++)
    {
        char *num = malloc(100000);
        scanf("%s", num);
        array[i] = num;
    }

    //print out array items and their memory addresses
    for(i=0; i<cases; i++)
    {
        printf("%d %s\n", i, array[i]);  //print (array index) (array value)
        printf("%d %p\n", i, (void*)&array[i]); //print (array index) (array address)
    }
    return 1;
}

You can as well use

char *num = calloc(100000, sizeof(char));

which is a little bit defensive. I don't know why you need 100000. You can do it dynamically using malloc. This will involve more work but is very robust.

What is hapenning in your code is that you store the string %s to the address of num which does not change, then you assign array[i] element to that address. Assigning in C is nothing else then storing the reference, you don't store the element itself- this would be waste of space. So as all of the array elements point to the address (only store the reference), the value in the adress change, thus so does the reference, that's why they all are changing to 2 (not 3 as you stated in your post).

Answer 5

It's because you are putting in every index of the array the same address (the address of char num[1000000];).

It's an error that will lead you to dynamic allocation (calloc, malloc, new, etc).

Cheers!

Answer 6

Replace

//store inputs in array
for(i=0; i<cases; i++)
{
    scanf("%s", &num);
    array[i] = &num;
}

with

array[0] = num;
//store inputs in array
for(i=0; i<cases; i++)
{
    scanf("%s", array[i]);
    array[i+1] = array[i] + strlen(array[i]) + 1;
}

to scan each string into the first available space in num[], and set the next element of array[] to point to the next available space. Now your printf() of the strings will work. The original was scanning every string into the start of num[].

Note: scanf() with unadorned %s is as bad as gets(), because it puts no limit on the amount of data that will be slurped in. Don't use it in real code.

Replace

    printf("%d %p\n", i, &array[i]); //print (array index) (array address)

with

    printf("%d %p\n", i, (void*)(array[i])); //print (array index) (array address)

to actually print the addresses stored in a[], rather than the addresses of the elements of a[]. The cast is required because %p expects a pointer-tovoid so you must provide one.

Answer 7

It's for such things that C++ seems to be made. User input parsing and dynamic allocations are done more securely, and in a breeze. I can't think of a system where you've got that kind of a user interface, where you couldn't switch to C++.

Of course, if this is only a testing excerpt from other code that suffers from the problem, then of course...

---

Your code suffers from several common mistakes for C beginners and things that should not be done that way nowadays.

If I understand correctly, you want to save sereval user input strings (your example output is a bit misleading, because you show only numbers).

You are preparing the array to hold all (cases count) pointers to strings, but you are only reserving memory for one string. You need to do that for every string, so cases. To keep things simple in terms of the "dynamic memory allocation" lesson, I recommend to do it that way: char* array[cases][10000]; That gives you cases strings of 10k characters.

You probably also do not want to have separate pointers to your array elements. This starts to make sense if you want to sort elements of an array when those elements are larger than the pointers itself. In that case, your gain in performance is not to have move (copy) large chunks, but only the pointers (usually 4 bytes). In your case, an int is also 4 bytes long. And you do not sort anyway :)

scanf() is dangerous, to say the least. In your second application, you are instructing it to write a string to the address of the array. This seems to be a simple mistake, but can lead to many problems. You probably want to do it that way: scanf("%d", &array[i]); (Unfortunately, I do not have a compiler at hand, so I am not 100% sure). drop the next line :)

---

Question to Markdown specialists: Why is it so damn impossible to have LISTS combined with CODE-blocks ?