Search code examples
ccs50caesar-cipher

CS50 Pset 2- ceasar cipher


It isn't showing what I want it to show that is the ciphered version of the input text but instead symbols, as I guess, looks kinda like a '?' comes out as the output in the terminal. could anyone help me in finding what I missed or did wrong?

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

int main(int argc, string argv[])
{
    if (argc == 2)
    {
        string ptext = get_string("plaintext: ");
        int key = (int) argv[1];
        printf("ciphertext: ");
        for (int i = 0, n = strlen(ptext); i < n; i++)
        {
            printf("%c", (( ptext[i] + key ) % 26);
        } 
        printf("\n");
    }
    else
    {
        printf("Invalid input. \n");
    }

}

I expect the output of 'hello' to be 'ifmmp' but instead, it doesn't.


Solution

  • This code is wrong:

    int key = (int) argv[1];
    

    argv is an array of string, which in CS50 is nothing more than an obfuscated char * pointer.

    Per 5.1.2.2.1 Program startup of the C standard:

    The function called at program startup is named main. The implementation declares no prototype for this function. It shall be defined with a return type of int and with no parameters:

        int main(void) { /* ... */ }
    

    or with two parameters (referred to here as argc and argv, though any names may be used, as they are local to the function in which they are declared):

        int main(int argc, char *argv[]) { /* ... */ }
    

    or equivalent; ...

    So argv[1] is a char * pointer value, which you then assign to an int value. That's taking the address of some memory (say the value in argv[1] is 0xFF0403220020480C) and trying to stuff it into the likely 4 bytes of the int variable key (which in this case would be assigned the truncated value 0x0020480C.)

    That's not what you appear to be trying to do.

    (IMO, your problem here is a perfect example of why CS50's obfuscation of char * with the string type is a tremendously bad idea. You simply can't understand C without understanding pointers and NUL-terminated char strings accessed via a char * pointer, and the obfuscation that string does makes that harder.)

    If you want to convert a string to a numeric value, you likely want something like strtol() (never use atoi() as it has no error checking and its use can invoke undefined behavior):

    char firstCharNotConverted;
    
    // set errno to zero as strtol()
    errno = 0;
    long key = strtol( argv[ 1 ], &firstCharNotConverted, 0 );
    
    // if errno is now non-zero, the call to strtol() failed (per Linux man page)
    // need to examine key and the contents of firstCharNotConverted
    // to figure out why
    if ( errno != 0 )
    {
        ...
    } 
    

    Proper headers omitted as an exercise for anyone trying to use this code ;-)

    Note that I used long for key, as you can't do complete and proper error checking on strtol() if you cast the return value to int.

    Error checking strtol() can be somewhat complex as the value returned (and assigned to key in the above code) can be any value and there are no values possible that aren't legitimate long values that strtol() can return to indicate an error, so for proper error checking you need to check the values of both errno and firstCharNotConverted to properly determine if an error did occur. The Linux man page states:

    Since strtol() can legitimately return 0, LONG_MAX, or LONG_MIN (LLONG_MAX or LLONG_MIN for strtoll()) on both success and failure, the calling program should set errno to 0 before the call, and then determine if an error occurred by checking whether errno has a nonzero value after the call.

    After that call to strtol(), you need to check if key is LONG_MIN, or LONG_MAX with errno equal to ERANGE for underflow or overflow, or if key is 0 you need to check the contents of firstCharNotConverted to determine why the conversion may have failed. Note that if key is zero and firstCharNotConverted isn't equal to argv[ 1 ], then input string was properly converted from zero.

    Your Ceaser cipher implementation is also wrong:

        for (int i = 0, n = strlen(ptext); i < n; i++)
        {
            printf("%c", (( ptext[i] + key ) % 26);
        } 
        printf("\n");
    

    will just print out characters with values from 0 through 25 - which aren't letters in the ASCII character set.

    There a numerous Ceaser ciphers questions already posted here, so I'm not going to write the code. See Caesar's Cipher Code for one example question.