CODEWITHSUNDEEP
cstruct
Ameer Shehadey
Ameer Shehadey

Reputation: 13

Why does printf behave this way when using struct?

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?

Upvotes: 0

Views: 113

Answers (1)

Mike Holt
Mike Holt

Reputation: 4612

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.

Upvotes: 1

Related Questions