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;
    }
}

Answer 2

This means the variable cyphertext is an array of characters and you need to terminate it with '\0' to make it a string. printf("%s", ...) requires a string and if you don't provide one your program exhibits undefined behavior.

The best option is to add the terminator right after your for-loop in main():

cyphertext[x] = '\0';

2nd best option is to initialize the variable:

char cyphertext[x + 1];
memset(cyphertext, 0, sizeof cyphertext);

3rd (least) best option is to tell printf() how many charterers to print. Many commonly used functions require a string (strlen(), strcmp(), strtok()), and only some also come in a variant that allows you to specify the length (strnlen(), strncmp()) but there is no strntok():

printf("cyphertext: %.*s\n", x, cyphertext);

Btw, prefer a better variable name than x for the length of your input string. n is a often used convention; length or size would work, too.