1

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?

Improve this question

4 Answers 4

Reset to default
5

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.

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

1 Comment

@Nico No problem, to make this a good lifetiome question, can you add the input string and a segment of indented code showing your expected output and what you actually got. That way its clearer for future readers.
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.

Improve this answer

Comments

2

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)
Improve this answer

3 Comments

my doubt was resolved before you answered, so i already accepted it. Yours is pretty clear and helpful, though. thanks a lot!! +1
I dont think the generated output from your first example is correct. The final print shows the names are different, but they should be the same.
@Ben, you're right, I accidentally copied the right answer twice. Thanks!
1

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.

Improve this answer

2 Comments

If you are giving this example you should mention as well that at the end of the scope the memory should be freed to avoid mem leaks.
@Ben ,thanks!! I thought that it was redundant to say that but I should be more specific in my answer ,I re-edited the answer.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.