0

I am writing a code while making the use of structures. I am new to structs so I am practicing to get used to it. Anyway while trying to use printf on a variable of type string which is a variable of a struct type, printf only prints '@' instead of the whole string.

...

void signPlayers(struct player players[9], int playersC) // players is an array declared on main and playersC is the size of it.
{
    for (int i = 0; i < playersC; i++)
    {
        char tname[22];
        printf("Please enter the name of the player #%d: \n",i+1);
        int res = scanf(" %s",&tname);
        while(res != 1)
        {
            printf("Please enter a valid name for player #%d: \n",i+1);
            res = scanf(" %s",&tname);
        }
        players[i].name = tname;
        printf("Player #%d signed as %s!\n\n",players[i].id,players[i].name); // this printf actually works fine
    }
}

int checkForWinner(struct player players[], int playersC)
{
    for (int i = 0; i < playersC; i++)
    {
        if (players[i].pos == 10)
            return 0;
        printf("%s\n",players[i].name); // prints "@" instead of the name
    }
    return 1;
}

...

so If I entered the name Joey, At first printf it actually prints "Joey", then when I call the checkForWinner function (Its called after signPlayers function), the printf now prints only "@" instead of the whole name again. What could be wrong?

10
  • 3
    Your program has undefined behavior because you store a pointer to memory on the stack and then try to use it after that stack has gone out of scope. Commented Mar 28, 2019 at 1:50
  • Wait I didn't get it, so what pointer did you talk about and when or why did the stack go out of scope? If I may ask. @paddy Commented Mar 28, 2019 at 1:56
  • 2
    @AmeerShehadey The assignment players[i].name = tname is assigning the address of a char array that exists only on the stack, and only for the duration of the call to signPlayers. You're then attempting to print the contents of that array, but tname went out of scope when the call to signPlayers completed, and now the memory that players[i].name is pointing to is just whatever random garbage was left over afterwards. Commented Mar 28, 2019 at 2:19
  • Okay, I think problem is solved after using the name member on the player struct as char name[] instead of char* name. Commented Mar 28, 2019 at 2:19
  • Also scanf(" %s",&tname); --> scanf("%21s",tname); Commented Mar 28, 2019 at 2:20

1 Answer 1

1

Your code is attempting to access stack memory after the function has returned. When you do that, you get just whatever garbage was left over after the function call, or perhaps some part of a new stack frame belonging to a different function. Either way, it's undefined behavior.

There are several problems with the assignment players[i].name = tname; in function signPlayers:

  1. It is assigning the address of the local array variable tname. This variable goes out of scope on every iteration through the loop. That means accessing that pointer after the loop will be undefined behavior.

  2. Even if tname was moved out of the loop's scope up to the function scope, it will still go out of scope as soon as the function returns. Either way, accessing it after the function call is undefined behavior.

  3. It's likely that the address of tname doesn't change from one iteration of the loop to the next. So that means that the .name member of every player in the players array will probably be identical (as well as being invalid).

There are many ways to fix this. Here are three ways:

  1. Make a copy of tname each time through the loop using strdup(tname), and assign that to players[i].name:

    players[i].name = strdup(tname);
    

    The strdup function allocates memory, so if you use this approach, you will need to remember to free the .name member of each player when you're done.

  2. Allocate memory dynamically to each players[i].name prior to calling signPlayers:

    for (i=0; i<playersC; ++i) players[i].name = malloc(22);
    signPlayers(players, playersC);
    // Don't forget to call free() on each .name member after you're done
    

    Inside signPlayers, you would then get rid of tname altogether and do:

    int res = scanf(" %s", players[i].name);
    

    Note: No need to do 22 * sizeof(char) here, since the C standard guarantees sizeof(char) == 1. Also, I used 22 because that's what OP's code uses to declare tname. However, it should be noted that scanf is less than ideal here, because it gives no way to limit the number of bytes written to the array, and thus no way to protect against buffer overflow. If you type a name longer than 21 characters (need to leave 1 byte for the null terminator), then you will overflow tname and your program will either crash or silently corrupt your data.

  3. Turn players[i].name into a sized char array instead of just a char pointer. In other words, statically allocate memory to each players[i].name. This has the advantage of not needing to call free, as you would need to do with methods 1 and 2. OP didn't show the definition of struct player, but this should suffice for example purposes:

    struct player {
      char name[22];
      // other stuff
    };
    

    And again, inside signPlayers, you would scanf directly into players[i].name instead of using tname.

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

Comments

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.