Caesar Cypher in C giving incorrect output

Question

Trying to create a Caesar Cypher in C and getting some troublesome output from lowercase letters that exceed 'z' when shifting letters

#include <stdio.h>

void confab(const char* inText, int shift, char* outText) 
{
    int i = 0;
    int j = 0;
    char character = '\0';
    while (inText[i] != '\0') {
        if (inText[i] >= 'A' && inText[i] <= 'Z') {
            character = inText[i] + shift;
            if (character > 'Z') {
                character = character - 'Z' + 'A' - 1;
            }
            outText[j] = character;
            j += 1;
        } else if (inText[i] >= 'a' && inText[i] <= 'z') {
            character = inText[i] + shift;
            if (character > 'z') {
                character = (character - 'z' + 'a' - 1);
            }
            outText[j] = character;
            j += 1;
        } else {
            outText[j] = inText[i];
            j += 1;
        }
        i += 1;
    }
}


int main(void)
{
    char buffer[60] = {0};
    char* s = "The quick brown fox jumps over the lazy dog.";
    confab(s, 13, buffer);
    printf("%s\n", buffer);
}

Is giving the output:

Gur d\202vpx oeb\204a sb\205 w\202zc\200 b\203re \201ur yn\207\206 qbt.

Instead of:

Gur dhvpx oebja sbk whzcf bire gur ynml qbt.

Suspecting my ASCII arithmetic is off in that if statement for the lowercase letters.

I suspect that you `char`s are signed and range from −128 to 127. Adding 13 to the later letters in the alphabet will go out of this range. You could probably do something like this: `c = 'a' + (c - 'a' + shift) % 26;` — M Oehm, Aug 18 '17 at 08:38
Just changing it to: `unsigned char character = '\0';` will work. — Andre Kampling, Aug 18 '17 at 08:40
while what I selected as the answer is correct and allowed for the correct output to be returned, the environment/conditions I was working under meant that I had to use mod in the end anyway so thank you for bringing it to my attention. — user8482543, Aug 19 '17 at 09:58

score 1 · Accepted Answer · answered Aug 18 '17 at 08:46

The problem with your code is that character variable is declared as signed 1-byte number.

So by looking at the ASCII table, a-z are in the range of 97-122. The problem is when you're trying to shift more than 5, it will overflow because signed 1-byte number can't exceed 127.

So by changing char character = '\0'; to unsigned char character = '\0'; will do the job.

You can refer here for the maximum number of integer for any byte - https://en.wikipedia.org/wiki/Integer_(computer_science)#Common_integral_data_types

Just for completeness: Signed overflow is undefined behaviour: https://stackoverflow.com/questions/18195715/why-is-unsigned-integer-overflow-defined-behavior-but-signed-integer-overflow-is. — Andre Kampling, Aug 18 '17 at 08:51

Caesar Cypher in C giving incorrect output

1 Answers1