Odd behavior from character array

Question

I am trying to write a helper program for Wordle which will display all of the letters which have not been eliminated and the letters which have been confirmed. I've broken the program into functions, and I'm having an issue with the confirmed_letters() function.

I have this array declared in the global scope: char* word[NUM_LETTERS]; and it's initialized like this:

for (int i = 0; i < NUM_LETTERS; i++) word[i] = new char(char(42));

Here's the function:

void confirmed_letters() {
    int num_let = 0; // the # of confirmed letters
    int temp_num; // holds the position of the confirmed letter
    char temp_letter;

    std::cout << "\n\nHow many letters have you confirmed?\n" <<
        "Enter only the letters whose positions are known. ";
    std::cin >> num_let;
    std::cin.ignore(1000, '\n');

    if (num_let == 0) return;
    std::cout << "Enter the integer position of the confirmed letter, " <<
        "followed by the letter. Press the enter (return) key between each entry.\n";
    for (int i = 0; i < num_let; i++) {
        //temp_num = -1; // I don't think this is needed
        std::cin >> temp_num;
        std::cin.ignore(1000, '\n');
        if (temp_num > 5 || temp_num < 1) {
            std::cout << "Invalid letter position. Enter an integer between 1 and 5.\n";
            i--;
            goto end;
        }
        std::cin >> temp_letter;
        std::cin.ignore(1000, '\n');
        word[temp_num - 1] = &temp_letter;
        end:
        display_word();
    }
    return;
}

display_word() is just a function to display the word.

Here's the output I got:

How many letters have you confirmed?
Enter only the letters whose positions are known. 3
Enter the integer position of the confirmed letter, followed by the letter. Press the enter (return) key between each entry.
1
o

o   *   *   *   *   
2
i

i   i   *   *   *   
3
e

e   e   e   *   *

So, for some reason, each additional letter is modifying the previous letters. I don't understand what is going on here. Each time the value of one of the elements of word[] is modified, it should only modify one element, but it's modifying all of them. Thoughts?

*array declared in the **global** scope* and `goto end;`. Huh. You must really like to suffer. — user4581301, Apr 21 '22 at 23:19
You can minimize the suffering by using `std::string` instead of character arrays. — Thomas Matthews, Apr 21 '22 at 23:19
`word[temp_num - 1] = &temp_letter;` stores the address of a local variable that will be reused on the next loop iteration and hold whatever new value the user inputs. The old value will be lost, so it'll look like all of the used slots store the same letter. Because they do. Don't store pointers to letters in `word`. Store the letters themselves. — user4581301, Apr 21 '22 at 23:21
Should I use `*(word[temp_num - 1]) = temp_letter;` instead? @user4581301 — Kyle Thompson, Apr 21 '22 at 23:23
Side note: Avoid storing the address of dynamically allocated variables (`new char(char(42));`) and automatically allocated variables (`char temp_letter;`) in the same pointer. The dynamically allocated variables eventually need to be destroyed and freed, `delete`ed, and you cannot `delete` an automatically allocated value because its management is automatic (ergo the name) and it will be destroyed and freed for you. In order to play mix-n-match with pointers to dynamic and automatic variables, you need to do what can be a lot of extra book-keeping. — user4581301, Apr 21 '22 at 23:32
@user4581301 Why are you even storing single `char` values in your array as `char*` pointers to begin with? Why not just declare the array as `char word[NUM_LETTERS];` instead? — Remy Lebeau, Apr 21 '22 at 23:35
@RemyLebeau That was a typo/uncorrected paste. Thanks for the fix. — user4581301, Apr 21 '22 at 23:37

user4581301 · Accepted Answer · 2022-04-21T23:40:22.170

word[temp_num - 1] = &temp_letter; stores the address of a local variable that will be reused on the next loop iteration and hold whatever new value the user inputs. The old value will be lost, so it'll look like all of the used slots store the same letter. Because they do. Worse, the variable goes out of scope at the end of the function and will no longer be valid. This is dangerous as the pointer lives on and might even give what looks to be correct results.

Solution: Don't store pointers to letters in word. Store the letters themselves.

char word[NUM_LETTERS];

Initialize like this:

for (int i = 0; i < NUM_LETTERS; i++) word[i] = '*'; 
    // use the character not the code. It's easier to understand the intent of 
    // * than the number 42 and not all systems will use 42 for *.

and store

word[temp_num - 1] = temp_letter;

Odd behavior from character array

1 Answers1