I keep running into a segmentation error that I can't seem to be able to fix

Question

For a course I'm taking I'm supposed to create a program that creates a simple ciphertext from plaintext using a key that the user inputs as a command line argument.

Here's the code, it's written in C

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

int main(int argc , string argv[])
{
    string key = argv[1];
    char keyL[2][26];
    char keyU[2][26];
    int normL = 97;
    int normU = 65;
    for (int i = 0;i <= 25;i++ , normL++, normU++)
    {
        keyL[0][i] = tolower(key[i]);
        keyU[0][i] = toupper(key[i]);
        keyL[1][i] = (char) normL;
        keyU[1][i] = (char) normU;
    }
    string plaint = get_string("plaintext:);
    string ciphert = "";
    int lengplain = strlen(plaint);
    for (int f = 0 ; f <= lengplain ; f++)
    {
        if (isupper(plaint[f]) == true)
        {
            for (int d = 0;d<=25;d++)
            {
                if (plaint[f] == keyU[0][d])
                {
                    ciphert[f] = keyL[1][d];
                }
            }
        }
        else if (islower(plaint[f]) == true)
        {
            for (int x = 0;x<=25;x++)
            {
                if (plaint[f] == keyU[0][x])
                {
                    ciphert[f] = keyL[1][x];
                }
            }
        }
        else
        {
            ciphert[f] = plaint [f];
        }
    }
    printf("ciphertext: %s\n" , ciphert);
}

This compiles but I run into a segmentation error when I run it. If you spot any logical errors please keep them to yourself. It's the segmentation error I am prioritizing to fix first.

Thanks!

Answer 1

Strings in c are not like strings in other languages. In c, strings are pointers to an array of characters in which the last character is the null terminator '\0'. When you declare the string ciphert = ""; you're essentially creating an length 1 array of characters with only the null terminator. The seg fault occurs when you try to access elements of ciphert that don't exist ciphert[f] = plaint[f]

You'll have to declare ciphert as an array of length lengplain + 1 which makes it the same length as plaintext (plus the null terminator) and will allow you to do ciphert[f] = plaint[f]. I recommend you take a look at some guides on strings in c.

Answer 2

string ciphert = "";

This will initialize the string with "". The problem? It is in a read-only section. And you write in this line: ciphert[f] = plaint [f]; to this array==> You write to a readonly section==>Crash. (Furthermore you should check argc for the number of args supplied, otherwise it will crash already in this line (keyL[0][i] = tolower(key[i]);)

Answer 3

there are several problems with the posted code. Here is a typical problem:

string plaint = get_string("plaintext:);

The parameter to get_string() has to be a pointer to a NUL terminated character array.

This parameter:

"plaintext:

is missing the trailing double quote: "

Have you ever tried to compile the posted code? I think not, because the compile would have failed due to this problem.

When compiling, always enable the warnings, then fix those warnings.

for gcc suggest:

gcc -c -g -Wall -Wextra -Wconversion -pedantic -std=gnu11

Note that other compilers use different options to produce the same results.

regarding;

int lengplain = strlen(plaint);

The function: strlen() returns a size_t, not an int

regarding;

string key = argv[1];

Never access beyond argv[0] without first checking argc to assure the expected command line parameter was actually entered by the user.

regarding:

int normL = 97;
int normU = 65;

it is a poor programming practice to use 'magic' numbers as they make the code much more difficult to understand, debug, etc. Suggest;

int normL = 'a';
int normU = 'A';

regarding:

char keyL[2][26];
char keyU[2][26];

the 'magic' number 26 is meaningless. Suggest (after the #include statements) define a meaningful name:

#define alphabetLen 26

Then use that meaningful name throughout the code.

You could save the readers of your code a lot of aggravation by changing:

for (int i = 0;i <= 25;i++ , normL++, normU++)
{
    keyL[1][i] = (char) normL;
    keyU[1][i] = (char) normU;

to

for ( int i = 0; i < alphabetLen; i++ )   
{
    keyL[1][i] =  normL + 'a';
    keyU[1][i] =  normU + 'A';

And:

int normL = 97;
int normU = 65;  

to:

int normL = 'a';
int normU = 'A';  

Of course, to eliminate a couple of the warnings, this

keyL[1][i] = normL;
keyU[1][i] = normU;

would be better written (and no compiler warnings) via:

keyL[1][i] = (char) normL;
keyU[1][i] = (char) normU;

regarding;

string ciphert = "";

This produces a pointer into readonly memory to a literal that is only 1 char long. Since it is in readonly memory, the code cannot write to it, nor offsets from it. Suggest:

string ciphert[ 26+1 ];