Search code examples
ccs50caesar-cipher

CS50 PSET 2 Caesar Erroneous Results


Thought I had completed Caesar, but when running CHeck50, my code fails for this reason: encrypts "barfoo" as "yxocll" using 23 as key output not valid ASCII text Log running ./caesar 23... sending input barfoo... checking for output "ciphertext: yxocll "...

Can anyone see anything wrong with my code? It seems to work fine for uppercase letters, but with lowercase letters and certain 'keys', I'm getting erroneous results, and can't spot why. Any help would be greatly appreciated.

Example: If I try to encipher 'foo' with a key of 17, it should return 'wff', but my code is returning 'w' only. With the code I have written it is saying to go to position 128 which isn't a letter, but my code is then saying if this is over 122, deduct 26. This equals and returns '102', - which is 'f'. Is it something to do with delete being assigned to 127

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

int main(int argc, string argv[])
{
  if (argc == 2) {
    int a = atoi (argv[1]);
    int b = a%26;
    printf("plaintext: ");
    //get string
    string s = get_string();
    printf ("ciphertext: ");
        //iterate through string
        for (int i=0, n =strlen(s); i<n; i++) {
            //check if character is a letter
            if ( isalpha (s[i])) {
                //check if letter is uppercase
                if (isupper (s[i])) {
                    //calculate position of character in ASCI by adding 'Key'. If character is over 90, decrease character location by 26
                    char c = (s[i] + b);
                    if (c > 90) {
                            char d = c - 26;
                            printf ("%c", d);
                    } else
                    printf("%c", c);
                } else
               //For lowercase letters. If character location is over position 122, decrease location by 26
                {
                    char e = (s[i] + b);
                    if (e>122) {
                            char f = e - 26;
                            printf("%c", f);
                    } else
                        printf("%c", e);
                }
            } else    //print non letters with no change made
            {
                printf ("%c", s[i]);
            }
        }
    }
printf ("\n");
return 0;

}


Solution

  • With lower-case letters, you may face overflow:

    char e = (s[i] + b);
    

    On your system, char is signed, which means it can take values from −128 to 127. Take lower-case z, which is ASCII 122. Shift it by 6 places or more, and you overflow your char. Overflow of signed integers is undefined behaviour. You can fix this by making your intermediate values ints:

    int e = (s[i] + b);