Caesar cypher debug: question marks and random char at the end of output text

Question

code

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

bool only_digits(string s);
char rotate(char c, int n);

int main(int argc, string argv[])
{
    if (argc < 2 || only_digits(argv[1]) == false || argc > 2) // check for th correct input
    {
        printf("usage: ./caesasr key \n");
    }
    else
    {
        int key = atoi(argv[1]);
        string plaintext = get_string("plaintext: ");
        int x = strlen(plaintext);
        char cyphertext[x + 1];
        for (int i = 0; i < x; i++)
        {
            cyphertext[i] = rotate(plaintext[i], key);
        }
        printf("cyphertext: %s\n", cyphertext);
    }
}

//make a function bool only_digits to check for input is a single digit between 0-9

bool only_digits(string s)
{
    if (s[0] > 47 && s[0] < 58 && strlen(s) == 1)
    {
        return true;
    }
    else
    {
        return false;
    }
}

//make a function char rotate(char c, int n) that rotate c by n on the alpahbet
// cout = (cin -65 + n)%26 +65 (uppercase)
// cout = (cin -97 + n)%26 +97 (lowercase)
// cout = cin (other)

char rotate(char c, int n)
{
    if (c > 64 && c < 91)
    {
        c = ((c - 65 + n) % 26 + 65);
    }
    if (c > 96 && c < 123)
    {
        c = ((c - 97 + n) % 26 + 97);
    }
    else
    {
        return c;
    }
    return c;
}

CLI outputs, question marks and chars added randomly to the cyphertext

I can't figure out where the question marks and randoms chars come from; it seems it only works with a 5-letter input; debug command finds everything works as intended until the last output letter is printed then suddenly random chars are added.

EDIT: changed line 21 to char cyphertext[x]; did not fix the problem

Answer 1

The output string is not null terminated: you added space for the null byte at the end, but you did not set it explicitly. The array cyphertext is uninitialized, so the character at cyphertext[i] may be anything and printf outputs it, then keeps reading and printing characters present in memory after the array, which is undefined behavior.

You should set the null terminator after the for loop with

            cyphertext[x] = '\0';

Also note these problems:

• for readability, you should use character constants such as 'A' instead of explicit ASCII codes like 65.
• only_digits should test all characters and accept strings longer than 1 byte for key values up to 25.

Here is a modified version:

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

bool only_digits(string s);
char rotate(char c, int n);

int main(int argc, string argv[])
{
    if (argc != 2 || only_digits(argv[1]) == false) // check for th correct input
    {
        printf("usage: ./caesasr key \n");
        return 1;
    }
    else
    {
        int key = atoi(argv[1]);
        string plaintext = get_string("plaintext: ");
        int x = strlen(plaintext);
        char cyphertext[x + 1];
        for (int i = 0; i < x; i++)
        {
            cyphertext[i] = rotate(plaintext[i], key);
        }
        cyphertext[x] = '\0';
        printf("cyphertext: %s\n", cyphertext);
        return 0;
    }
}

//make a function bool only_digits to check for input is a string with one or two digits

bool only_digits(string s)
{
    int len = strlen(s);
    if (len < 1 || len > 2)
    {
        return false;
    }
    for (int i = 0; i < len; i++)
    {
        if (s[i] < '0' || s[i] > '9')
        {
            return false;
        }
    }
    return true;
}

//make a function char rotate(char c, int n) that rotate c by n on the alpahbet
// cout = (cin - 'A' + n) % 26 + 'A' (uppercase)
// cout = (cin - 'a' + n) % 26 + 'a' (lowercase)
// cout = cin (other)

char rotate(char c, int n)
{
    if (c >= 'A' && c <= 'Z')
    {
        return (c - 'A' + n) % 26 + 'A';
    }
    else if (c >= 'a' && c <= 'z')
    {
        return (c - 'a' + n) % 26 + 'a';
    }
    else
    {
        return c;
    }
}