Segmentation Fault While Manipulating String

Question

I'm trying to create a Ceasar Cipher. For this test, I used a key of 1 and plain text of "hello." When doing so, I get the error message "Segmentation fault (core dumped)". I know this means I am trying to access an illegal memory location, and it's happening while calling the "encrypt" function, but that's all I know.

Depiction of the error I'm getting while debugging.

And here is my code.

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

bool isValidKey(string);
string encrypt(string, int);

string c;

int main(int argc, string argv[])
{
    if (argc != 2 || isValidKey(argv[1]) == 0)
    {
        printf("Useage: ./caesar key\n");
        return 0;
    }
    string p = get_string("plaintext: ");
    c = encrypt(p, atoi(argv[1]));
    printf("%s", c);
    return 0;
}

bool isValidKey(string key)
{
    for (int i = 0; i < strlen(key); i++)
    {
        if (isdigit(key[i]) == 0)
        {
            return false;
        }
    }
    return true;
}

string encrypt(string plain, int k)
{
    for (int i = 0; i < strlen(plain); i++)
    {
        if (isalpha(plain[i]) != 0)
        {
            if (islower(plain[i]) != 0)
            {
                c[i] = ((plain[i] - 97 + k) % 26) + 97;
            }
            else
            {
                c[i] = ((plain[i] - 65 + k) % 26) + 65;
            }
        }
    }
    return c;
}

string c; declares a pointer, it doesn't allocate any memory for it. — Barmar
– Barmar, Commented Jul 12, 2022 at 20:36
Instead of 97, write 'a'. Similarly for 65. The magic number 26 should be replaced with something like SIZE_OF_ALPHABET or 'z' - 'a' + 1. — William Pursell
– William Pursell, Commented Jul 12, 2022 at 20:40

Vlad from Moscow · Accepted Answer · 2022-07-12 20:58:42Z

The variable c declared in the file scope

string c;

is a null pointer. The above declaration is equivalent to

char *c;

or in fact to

char *c = NULL;

So using the null pointer within the function encrypt like

c[i] = ((plain[i] - 97 + k) % 26) + 97;

invokes undefined behavior.

Also it is a bad idea when a function like encrypt depends on a global variable.

Also using magic numbers like for example 97 makes the program unclear.

As the function parameter does not have the qualifier const then it will be logical to convert the passed string in place.

Pay attention to that the user can pass a very big integer value as a key. In this case your function will invoke undefined behavior.

So the function can look like

string encrypt( string plain, int k)
{
    k = k % 26;

    for ( size_t i = 0, n = strlen( plain ); i < n; i++ )
    {
        if ( isalpha( ( unsigned char )plain[i] ) )
        {
            if ( islower( plain[i] ) )
            {
                plain[i] = ( plain[i] - 'a' + k ) % 26 + 'a';
            }
            else
            {
                plain[i] = ( plain[i] - 'A' + k ) % 26 + 'A';
            }
        }
    }

    return plain;
}

The variable c should be removed and in main it is enough to write

string p = get_string( "plaintext: " );
puts( encrypt( p, atoi( argv[1] ) ) );

Barmar · Accepted Answer · 2022-07-12 20:38:47Z

0

You need to allocate memory for c.

string encrypt(string plain, int k)
{
    c = malloc(strlen(plain) + 1);

    for (int i = 0; i < strlen(plain); i++)
    {
        if (isalpha(plain[i]) != 0)
        {
            if (islower(plain[i]) != 0)
            {
                c[i] = ((plain[i] - 97 + k) % 26) + 97;
            }
            else
            {
                c[i] = ((plain[i] - 65 + k) % 26) + 65;
            }
        }
    }
    return c;
}

And in main() you should add free(c); before return 0;

edited Jul 12, 2022 at 20:38

answered Jul 12, 2022 at 20:37

Barmar

789k57 gold badges555 silver badges669 bronze badges

6 Comments

Eugene Sh. Over a year ago

You are not terminating c.

Barmar Over a year ago

I was just adding that

Tofu Over a year ago

How do you know string is a typedef for char *?

Barmar Over a year ago

Because I know it's a typedef in cs50.h.

Tofu Over a year ago

I really don't understand how teaching C with non standard headers and libraries is helpful.

|

Collectives™ on Stack Overflow

Segmentation Fault While Manipulating String

2 Answers 2

Comments

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related