Array of structs inside a loop

Question

I'm learning C and i'm playing around with structures, but i have found some behaviour i can't explain, and i'd like to know why it happens.

This is my code:

struct my_struct{
  char *name;
};

int main()
{

  struct my_struct arr[3];
  int i = 0;
  char str[10];

  while (i<3)
  {
    fgets(str, 10, stdin);
    arr[i].name = str;
    printf("Array number %d: %s", i, arr[i].name); 
    i++;
  }

  printf("1 - %s\n2 - %s\n3 - %s", arr[0].name, arr[1].name, arr[2].name);

  return 0;
}

My input :

test1
test2
test3

Expected output :

Array number 0: test1

Array number 1: test2

Array number 2: test3
1 - test1

2 - test2

3 - test3

Resulting Output:

Array number 0: test1

Array number 1: test2

Array number 2: test3
1 - test3

2 - test3

3 - test3

Resulting Output:

The issue is, as long as the while loop keeps running, it seems to be alright; however, when it exits, it seems to set each of the "name" values of the structs in the array to the last one's.

If, once out of the loop and before the last printf(), I set the name of the last struct in the array manually, that is the only one which is updated, but the previous structs' names are still set to the last one entered inside the loop.

I imagine i'm missing sth about memory management like flushing the buffer before calling to fgets() again or sth, but can't figure out what is happening. Does anyone know what this is about?

Answer 1

This is what you expect, think about it this way, you have char str[10], this is the memory in which your string is stored. When you set each of the array names with arr[i].name = str you are pointing the char * name at this memory. So here is what your for loop is doing:

0. str = [];         arr[0].name = NULL; arr[1].name = NULL; arr[1].name = NULL;
1. str = [string1];  arr[0].name = str;  arr[1].name = NULL; arr[1].name = NULL;
2. str = [string2];  arr[0].name = str;  arr[1].name = str;  arr[1].name = NULL;
3. str = [string3];  arr[0].name = str;  arr[1].name = str;  arr[1].name = str;

So by the end of the loop, all of the arr.name pointers point at string and you have edited the string each time. If you want the individual arr elements to store their own string then you are better off doing this:

struct my_struct{
  char name[10]; // Note how each instance of `my_struct` now stores its own string.
};

int main() {
  struct my_struct arr[3];
  int i = 0;

  while (i<3) {
    fgets(arr[i].name, 10, stdin);
    printf("Array number %d: %s", i, arr[i].name); 
    i++;
  }

  printf("1 - %s\n2 - %s\n3 - %s", arr[0].name, arr[1].name, arr[2].name);

  return 0;
}

Live example

As a final note you should avoid using fgets (see here). Prefer getline instead.

Answer 2

You cannot copy a C string like that arr[i].name = str.

What you are doing is setting every name pointer to the same address, represented by str. So, when you call printf, each name points to the same string, str, and printf simply prints str three times.

If you want to copy a string, use strcpy. Also, you need to allocate the memory for your name variables.

Answer 3

Because this is C, you need to manage all memory construction/destruction/management yourself.

If you are just learning C, it's probably better that you stick to char arrays for now, rather than diving straight into pointers, unless you know what you're doing, or at least without doing a little bit of research/learning beforehand.

Arrays are different than pointers, and there is a post here detailing the differences.

Specific to your problem, you can more clearly understand what's going on by adding a little bit more output to your code. Using the %p format specifier, you can print out the pointer location of a variable.

There are several ways to debug a C program to figure out this stuff, like using gdb on Linux or Visual Studio on Windows, but this is the easiest way to show on here.

Adding some debugging output to your program, you get this:

#include<stdio.h>

struct my_struct{
  char *name;
};

int main()
{

  struct my_struct arr[3];
  int i = 0;
  char str[10];

  while (i<3)
  {
    fgets(str, 10, stdin);
    arr[i].name = str;
    printf("Array number %d: %s", i, arr[i].name); 
    printf("  %p\n", arr[i].name);
    i++;
  }

  printf("\n1 - %s (%p)\n2 - %s (%p)\n3 - %s (%p)", 
    arr[0].name, arr[0].name, 
    arr[1].name, arr[1].name,
    arr[2].name, arr[2].name);

  return 0;
}

This results in the following output (given john, jacob, and jingle as input):

Array number 0: john
  0xfff8d96a
Array number 1: jacob
  0xfff8d96a
Array number 2: jingle
  0xfff8d96a

1 - jingle
 (0xfff8d96a)
2 - jingle
 (0xfff8d96a)
3 - jingle
 (0xfff8d96a)

From this, we can see that you are clearly overwriting the same memory address every time. The reason for this is because name is assigned to str, and more pedantically, name is being set to the location at which the char* str exists in memory, which likely isn't going to change. This is essentially the same as doing x = 3 in a loop three times.

To fix this, you need to do two things.

1. Allocate each arr[i].name instance before you use it. This can be achieved using malloc or calloc from stdlib.h.
2. Copy the input retrieved from stdin into arr[i].name. This can be achieved using strcpy or strncpy (preferred) from string.h

After applying the fix (two lines of code), your loop will look like this:

while (i<3)
{
  fgets(str, 10, stdin);

  // Allocate 10 (most likely) bytes of memory to arr[i].name
  // And also clear out that memory space      
  arr[i].name = (char*)calloc(10, sizeof(char));

  // Safely copy the data (max 10 chars) from 'str' into 'arr[i].name'
  strncpy(arr[i].name, str, 10);

  printf("Array number %d: %s", i, arr[i].name); 
  printf("  %p\n", arr[i].name);

  i++;
}

After applying the fix, the end result is this:

Array number 0: john
  0x89a0008
Array number 1: jacob
  0x89a0018
Array number 2: jingle
  0x89a0028

1 - john
 (0x89a0008)
2 - jacob
 (0x89a0018)
3 - jingle
 (0x89a0028)

Answer 4

You have to allocate memory for char *name of each struct:

while (i<3)
  {
    fgets(str, 10, stdin);
    arr[i].name=(char *)malloc(10*sizeof(char));
    strcpy(arr[i].name,str);

    printf("Array number %d: %s", i, arr[i].name); 
    i++;
  }

Note that, by this way when you're done using the array[i] you should free(array[i].name) ,for each i<3 to avoid memory leaks.