CS50 Pset 2 Substitution - Array size errors

Question

I'm using a for loop to assign chars to each element of an array one at a time. The array has been assigned a size and the for loop runs the same number of iterations as the array size, but I find that as soon as I assign a value to the 8th element of the array (cypstring[7]), the size of the array increases to 14, even though I may have given it a size of say 10, assigning random characters to the last 4 elements.

int ciphertext(char cypherchk[], string plainstring)
{
   int plainlength = strlen(plainstring);
   char cypstring[plainlength];
   for (int n = 0; n < plainlength; n++)
   {
        int asc = toupper(plainstring[n]) - 65;
        if (isupper(plainstring[n]) != 0)
        {
            cypstring[n] = toupper(cypherchk[asc]);
        }
        else if (islower(plainstring[n]) != 0)
        {
            cypstring[n] = tolower(cypherchk[asc]);
        }
        else
        {
            cypstring[n] = plainstring[n];
        }
    }

    printf("ciphertext: %s\n", cypstring);

    return 0;
}

Any ideas about what is going on?

Answer 1

It looks like a common newbie problem. Don't worry, we all learn about this the same way. ;-)

An educated guess would be that the code is printing the encrypted string correctly, followed by likely random characters.

When you create a string array, you ALWAYS have to remember to allocate one extra char or byte to the length to hold the end of string (EOS) marker of 0x00. This is very important because functions like strlen() and printf() depend on its existence. (There are some cases where this is not needed, as when processing characters individually throughout the code, but it's still a best practice to do. You never know when the code could be changed to something requiring that EOS marker!)

Here's why. These functions look for the EOS marker to determine the actual end of the string. If it's not there, the function will continue searching beyond the end of the actual string for 0x00 until it finds it. The result is totally unpredictable. It will depend on when the next 00 byte is found in memory. It could be the next byte, or it could be megabytes later in memory. It all depends on what the program has stored in it's allocated memory.

Try adding 1 to the length that you're allocating and manually set the EOS marker:

char cypstring[plainlength + 1]; cypstring[plainlength] = 0x00;

Hopefully, this will cure your issues.

Coding note:

        if (isupper(plainstring[n]) != 0)

It isn't necessary to do a comparison here. In fact, this does 3 actions where 2 is all that is required. First, it calls isupper to check if it is in fact an uppercase letter. If it is, it returns a positive number (not necessarily 1, but a power of 2). Second, it compares the return value to 0. Third, it checks the result of that comparison for true/false state.

Instead, simplify it to the call. The return value will be analyzed for true/false state.

    if (isupper(plainstring[n]))

No need for an extra comparison. Consider this for all of the issomething function calls. ;-)