My constant is changing, and I dont know why, or even how

Question

I'm making a program that basically "ciphers" a text, by substituting the letters by other ones. So you basically run the program and enter a distribution, then enter the text you want to cipher, and it gives you back the cipher.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <cs50.h>
#include <ctype.h>

string make_lower(string word);

int main (int argc, string argv[])
{
    //we examine the input
    if (argc < 2 || argc > 2)
    {
        printf("Usage: ./substitution key\n");
        return (1);
    }
    
    if (strlen(argv[1]) != 26)
    {
        printf("Key must contain 26 characters.\n");
        return (1);
    }
    
    if (strlen(argv[1]) == 26)
    {
        for (int i = 0; i < 26; i++)
        {
            if (isdigit(argv[1][i]))
            {
                printf("Key must onlyontain alphabetic characters.\n");
                return (1);
                exit(0);
            }
        }
        
        for (int i = 0; i < 26; i++)
        {
            for (int o = i + 1; o < 26; o++)
            {
                if (argv[1][i] == argv[1][o])
                {
                    printf("Key must not contain repeated characters.\n");
                    return (1);
                    exit(0);
                }
            }
        }
        
        //we prompt the user for the words to transcribe
        const string text = get_string("plaintext:  ");
        
        //we set everithing in lower caso to compare it, and set a new variable to store the original text so we can convert the 
        // upper cases back later
        string ntext = text;
        
        printf("%s\n", text);
        
        argv[1] = make_lower(argv[1]);
        
        ntext = make_lower(ntext);
        
        printf("%s\n", text);
        
        string alphabet = "abcdefghijklmnopqrstuvwxyz";
        
        //we substitute the text to the desired outcome
        for (int i = 0, n = strlen(ntext); i < n; i++)
        {
            for (int o = 0; o < 26; o++)
            {
                if (ntext[i] == alphabet[o])
                {
                    ntext[i] = argv[1][o];
                }
            }
        }
        
        printf("%s\n", text);
        
        printf("ciphertext: ");
        
        for (int i = 0, n = strlen(ntext); i < n; i++)
        {
            if (isupper(text[i]))
            {
                printf("%c", toupper(ntext[i]));
            }
            else
            {
                printf("%c", ntext[i]);
            }
        }
        
        printf("%s\n", text);
        
        printf("\n");
        
        return(0);
        
        exit(0);
    }
}

string make_lower(string word)
{
    for (int i = 0; i < strlen(word); i++)
    {
        word[i] = tolower(word[i]);
    }
    
    return(word);
}

So my problem is on the output of the code, since the thing is that if the text you wanna cypher is for example "Hello", it should come out dor example like "Ktlly", but instead my output is "ktlly", so the upper cases doesn't show in the result.

When you enter the code you want to cipher the program stores it in a constant string, and then creates a new variable and sets it equal to the text you entered, then the program converst that new variable to lower case so it can be compared, and at the end when we got the text cyphered, I try to turn those characters I want to upper case again by making an if statement (in line 82), but it just doesn't. I tried to see why, and thats why I set to print the constant in the lines 56, 62, 78 and 94, to see why it wasn't working, and it turns out that the variable was changing, even tho is suposed to be a constant. At first in line 56 its still the original text; then in line 62, its the same text, but in lower case; then in line 78 and 94, it turns out in modified itself to be the ciphered version of the text.

So thats basically it, I dont know why it is happening. At least to me, the code seems right, my theory is that it has something to do with the function, or it has to do with the big "if" statements the hole code is built in. Thanks for reading throught all this.

Answer 1

The following comment is wrong:

//we set everithing in lower caso to compare it, and set a new variable to store the original text so we can convert the 
// upper cases back later
string ntext = text;

The identifier string defined in cs50.h is nothing more than a typedef for the data type char *. This means that a variable of type string does not actually contain a string, but merely points to a string.

In the line quoted above, you are merely copying the pointer, but not the actual string. Since ntext and text now both point to the same string, when you modify the string pointed to by ntext, you are also modifying the string pointed to by text. This does not seem to be what you want.

If you want to create a copy of the string, instead of only copying the pointer, you must first allocate memory for the new string using malloc, and then you can copy the string using strcpy:

//allocate memory for copy of the string
string ntext = malloc( strlen(text) + 1 ); //add 1 for the terminating null character of the string

//copy the string
strcpy( ntext, text );

Most compilers also support the function strdup, which handles allocation of the memory and copying of the string in a single function call. If your compiler supports that function, then you can use it instead:

string ntext = strdup( text );

Note that all memory allocation functions, such as malloc and strdup, may fail, for example when the operating system is out of memory. In that case, the functions malloc and strdup will return a NULL pointer. If you use such a NULL pointer without checking it beforehand whether it is NULL, then your program will likely crash. For this reason, the code I posted above is actually bad, because it is not checking for NULL before dereferencing the pointer.

Therefore, the following code would be better, which checks the return value for NULL, and prints an error message and quits the program, if the memory allocation fails:

For malloc/strcpy:

//allocate memory for copy of the string
string ntext = malloc( strlen(text) + 1 );

//verify that allocation was successful
if ( ntext == NULL )
{
    fprintf( stderr, "memory allocation error!\n" );
    exit( EXIT_FAILURE );
}

//copy the string
strcpy( ntext, text );

For strdup:

string ntext = strdup( text );

if ( ntext == NULL )
{
    fprintf( stderr, "memory allocation error!\n" );
    exit( EXIT_FAILURE );
}

Answer 2

make_lower overwrote its argument. You need to copy the string. Consider using strdup().

I'm kind of surprised this isn't raising a compiler warning.

string make_lower(string word)
{
    word = strdup(word);
    //...