The for loop rewrites all values of array in structure C

Question

I have a problem with assigning values to a string array which is a field in my struct. In this case, I am also using an array of this structs. When I assign values without for loop, everything works fine. But the problem occurs in the for loop. I have printed all assignments and they are OK. But the problem actually is, that the every assignment actually rewrites all array positions in all structures of that field.

struct ship 
{
int length;
char *fields[4];
};

struct ship shipPositions[6];

for(int h=0; h < length; h++) //goes from 0 to 3 for first two values
        {

            char c = getCharFromInt(begin + h); //my function which works fine; begin = 2;
            char coord[3] = "$";
            append(coord, c); // also works fine
            append(coord,ships[i][1]);
            shipPositions[i].fields[h] = coord;
            printf("shipPositions[%d].fields[%d] = %s\n", i,h,shipPositions[i].fields[h]);


        }

printf("Position: %s\n",shipPositions[0].fields[0]);
printf("Position: %s\n",shipPositions[0].fields[1]);

/* it prints assignments and then on the first two values after for loop
shipPositions[0].fields[0] = $C2
shipPositions[0].fields[1] = $D2
shipPositions[0].fields[2] = $E2
shipPositions[1].fields[0] = $F0
shipPositions[1].fields[1] = $G0
shipPositions[1].fields[2] = $H0
shipPositions[5].fields[0] = $H8
shipPositions[5].fields[1] = $I8
Position: $I8
Position: $I8
*/

And when I simulate first two cases of for loop, everything is also fine:

char c1 = getCharFromInt(2 + 0);
char coord1[3] = "$";
append(coord1, c1);
append(coord1,ships[0][1]);
shipPositions[0].fields[0] = coord1;
printf("shipPositions[%d].fields[%d] = %s\n", 0,0,shipPositions[0].fields[0]);

char c2 = getCharFromInt(2 + 1);
char coord2[3] = "$";
append(coord2, c2);
append(coord2,ships[0][1]);
shipPositions[0].fields[1] = coord2;
printf("shipPositions[%d].fields[%d] = %s\n", 0,1,shipPositions[0].fields[1]);

printf("Position: %s\n",shipPositions[0].fields[0]);
printf("Position: %s\n",shipPositions[0].fields[1]);

/*
shipPositions[0].fields[0] = $C2
shipPositions[0].fields[1] = $D2
Position: $C2
Position: $D2


*/

It is because you've got a pointer to a variable that dies at the end of the block. Also, perhaps you shouldn't strings for fields like that, instead use a structure of struct Position { int x, y; } say. — Antti Haapala
– Antti Haapala, Commented Oct 27, 2017 at 10:21
Thanks Antti for the answer. Can you please write me how should I fix this loop. Thanks in advance. — Stefan Puhalo
– Stefan Puhalo, Commented Oct 27, 2017 at 10:24
I reopened the question though. This answer does tell why it happens and how to fix in general case, but the general case is not really appropriate here. Use a struct instead of a string for coordinates. — Antti Haapala
– Antti Haapala, Commented Oct 27, 2017 at 10:28

Chris Turner · Accepted Answer · 2017-10-27 11:05:42Z

0

There's 2 problems with your code.

Firstly when you declare char coord[3], you're creating a string of 2 characters long because you also need to include a NUL character to terminate the string. But you're storing 3 characters in it, so you need to declare it as char coord[4].

You don't provide the code for append, but you do need to remember to assign the NUL character if it isn't doing it for you ie coord[3]='\0'

Secondly, coord only exists as a variable for each iteration of the loop, but the memory it occupies will still remain. When you assign coord to shipPositions[i].fields[h] you're assigning a pointer to the memory, not the actual contents of the string. When an iteration of the loop finishes, coord ceases to exist and your code can use the memory it occupied for other things (like the next instance of coord) overwriting the value you expect to find in shipPositions[i].fields[h].

You should declare fields to be an array of arrays like char fields[4][4] and then when you want to stash the contents of coord you would use strcpy ie strcpy(shipPositions[i].fields[h],coord). That way you "own" the memory you're storing the value in and it won't get overwritten once coord no longer exists.

answered Oct 27, 2017 at 11:05

Chris Turner

8,1421 gold badge16 silver badges18 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Stefan Puhalo Over a year ago

Thanks Chris, I appreciate it !

Collectives™ on Stack Overflow

The for loop rewrites all values of array in structure C

1 Answer 1

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related