C array elements poiting to same location

Question

Need some help please. I have the following code.

    char *lines[100];
    int i;

    for (i = 0; i < 100; i++)
    {
        char temp[10];
        _itoa_s(i, temp,10);
        char result[10] = "test";
        strcat_s(result, temp);

        lines[i] = (char*)malloc(sizeof(char));
        lines[i] = result;
    }

    for (i = 0; i < 100; i++)
    {
        cout << lines[i] << endl;
    }

Why does it print :

test99
test99
test99
...

It turns out that char result[10] will point to the same memory location. Why ? Was expecting something like this :

test1
test2
test3
...

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, May 04 '17 at 09:29
@SouravGhosh ....or are you using malloc in c++...? ;) malloc cast is required with c+ compiler. — LPs, May 04 '17 at 09:30
I come from C# background. I m just interested in C logic and memory management ... not the details ( cout vs printf ) — doremifasolasido, May 04 '17 at 09:30
@doremifasolasido The program has undefined behavior. For starters the memory is allocated incorrectly for the array elements and the variable result is a local variable that is declared in a code block and is not alive outside it. — Vlad from Moscow, May 04 '17 at 09:33
`result` scope is for loop scope. I think it is UB to access its address after the loop. — LPs, May 04 '17 at 09:35
@doremifasolasido You do know that C doesn't have `cout`, right? That's a C++ thing. And as for the solution, `lines[i] = (char*)malloc(sizeof(char)); lines[i] = result;` → `lines[i] = malloc(strlen(result) + 1); strcpy(lines[i], result);` — Spikatrix, May 04 '17 at 09:36
@CoolGuy (probable reply :P) why you're bothering, I don't mind whether it's there or not.... — Sourav Ghosh, May 04 '17 at 09:38
@LPs Using malloc in C++ means something is fundamentally wrong with your program. It is not compatible with classes, nor with `new/delete`. — Lundin, May 04 '17 at 09:47
@Lundin For sure. I tried tagged the question as C++-no-oop, as the code is, but was rejected. BTW I meant using c++ compiler to compile the code. — LPs, May 04 '17 at 09:49

Jabberwocky · Accepted Answer · 2017-05-04T10:00:57.990

The first line here is a basically NOOP (actually it is creating a memory leak because you throw away the pointer returned by malloc by overwriting it on the next line).

lines[i] = (char*)malloc(sizeof(char));  // this is a basically a NOOP
lines[i] = result;

It's more or less like writing:

foo = 5;
foo = result;

So your code ends up like this:

for (i = 0; i < 100; i++)
{
    char temp[10];
    _itoa_s(i, temp,10);
    char result[10] = "test";
    strcat_s(result, temp);

    lines[i] = result;  // copying just the pointer,
}

So all lines[i] contain the pointer to the same memory location result.

Also the scope of result is limited to the part between {}, so once the for loop is terminated, result is likely to be overwritten at the next occasion.

You need this:

char *lines[100];

for (int i = 0; i < 100; i++)
{
    char temp[10];
    _itoa_s(i, temp,10);
    char result[10] = "test";
    strcat_s(result, temp);

    lines[i] = (char*)malloc(sizeof(char) * strlen(result)   + 1);
                                         // ^string length     ^space for NUL terminator

    strcpy(lines[i], result);  // actually copying the the string (not only the pointer)
}

You also need to free the allocated memory once you're done with it:

for (i = 0; i < 100; i++)
{
    free(lines[i]);
}

It is not really a no-op, because it has the side-effect of creating a fat memory leak. In the OP's code, the compiler will blindly follow the instructions: "Aha, this line must get executed because the programmer wants a memory leak here". Unlike the code `foo = 5; foo = result;` where the first assignment may get optimized away. — Lundin, May 04 '17 at 09:50

Sourav Ghosh · Answer 2 · 2017-05-04T09:35:06.087

2

In your code, due to

 lines[i] = result;

all the pointers end up pointing to the same result, the last one.

You may want to use strcpy() if you're interested in the content.

But wait, there's another problem. You're written

 lines[i] = (char*)malloc(sizeof(char));

which allocates 1 byte of memory, which is here, good for nothing. Before you use the pointer as destination in strcpy(), you need to make sure that the pointer points to sufficient memory, including the terminating null, to hold the concatenated string.

Advice: The devil is in the details. perform a careful study of the language before performing trial and error.

edited May 04 '17 at 09:35

answered May 04 '17 at 09:32

Sourav Ghosh

133,132
16
183
261

Doesn't the scope of result matter here? It is UB to access result address after lop scope, isn't it? – LPs May 04 '17 at 09:34
1

@LPs hah, right of course.... but probably with addition of strcpy(), that will be avoided and that's the target. – Sourav Ghosh May 04 '17 at 09:36

C array elements poiting to same location

2 Answers2