Caesar for pset2

Question

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

string plain_text;
string cipher_text;
int key = 0;
int main(int argc, string argv[])
{
    if (argc != 2)
    {
        printf("Invalid Key.\n");
        return 1;
    }
    key = atoi(argv[1]);
    for(int i = 0, len = strlen(argv[1]); i < len; i++)
    {
        if(argv[1][i] < 48|| argv[1][i] > 57)
        {
            printf("Usage: ./caesar key\n");
            return 1;
        }
    }

    if(key < 0)
    {
        printf("Key cannot be negative.\n");
        return 1;
    }
    plain_text = get_string("plaintext:  ");
    for(int i = 0, len = strlen(plain_text); i < len; i++)
    {
        if(islower(plain_text[i]))
        {
            cipher_text[i] = (plain_text[i] - 97 + key)%26 + 97;
        }
        if(isalpha(plain_text[i]))
        {
            cipher_text[i] = (plain_text[i] - 65 + key)%26 + 65;
        }
    }
    printf("ciphertext: %s",cipher_text);
    return 0;

}

Says segmentation core dump when I type in plaintext. p

Answer 1

You're right, the code doesn't allocate enough memory for the strings. In fact, it didn't allocate anything.

The declarations are as follows:

    string plain_text;
    string cipher_text;

In both cases, the code doesn't assign anything to them. the result is that there's no space allocated to the content of the strings, just one byte for the end of string (EOS) marker.

When creating a string, aka, a character array (you'll learn more in the next couple weeks), memory has to be allocated either by explicitly setting the length, or by assigning an actual string to the variable. Otherwise, the program will simply assign an addrtess (and a single byte) to that string.

In order to do this successfully, you'd need to do something like this:

    string plain_text = get_string("plaintext:  ");

This will simultaneously create the variable "plain_text:", allocate sufficient space to it (the length of the string plus 1 byte for the EOS marker), and will populate the memory with the string that the user typed.

Next, you'd need to do something similar with cipher_text. I'll leave that for you to figure out.

Having said all that, there's another issue that needs to be fixed - the use of global variables.

Global variables are vars that are created outside of main or any function. There are 3 in this code - plain_text, cipher_text and key. It is a universally bad practice to use GLOBAL vars in code unless absolutely necessary or there's a compelling reason. Globals are to be avoided because they frequently lead to code bugs and problems, especially when a local variable is created using the same name, thus creating a shadow variable (stacked vars with the same name.)

Global vars can be a useful tool when they're justified, but that is usually not the case. Avoid creating them whenever possible.

In this program there's absolutely no need for globals, since all the code is contained in the main function.

There's another problem with globals. They can't be initialized with another variable or user input at creation - only with constants that can be precompiled (ie, int key = 0;.)

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)