Search code examples
ccommand-line-argumentsundefined-behaviorcs50argv

Caesar segmentation fault in c language


I'm working on the second homework Caesar of CS50. It seems most of my review is correct except the last one. I cannot handle the situation of lacking argv[1], which means if I only type ./caesar, it will segmentation fault. Why?

#include <stdio.h>
#include <string.h>
#include <cs50.h>
#include <stdlib.h>
#include <ctype.h>
#include <math.h>
bool check;

int key (int c, string v[]);

int main (int argc, string k[])
{
    key (argc, k);
    if ((check))
    {
        string p = get_string("plaintext: ");
        int n = strlen (p);
        char f[n];
        printf ("ciphertext: ");
        for (int i = 0; i < n; i++)
        {
            if (isalpha (p[i]))
            {
            if (p[i] >= 'A' && p[i] <= 'Z')
            {
                if ((p[i] + x) > 90)
                {
                f[i] = ((p[i] + x) % 91) + 65;
                printf ("%c", f[i]);
                }
                else
                {
                    f[i] = ((p[i] + x) % 91);
                    printf ("%c", f[i]);
                }
            }
            else if (p[i] >= 'a' && p[i] <= 'z')
            {
                if ((p[i] + x) > 122)
                {
                f[i] = ((p[i] + x) % 123) + 97;
                printf ("%c", f[i]);
                }
                else
                {
                    f[i] = ((p[i] + x) % 123);
                    printf ("%c", f[i]);
                }
            }
            }
            else
            {
                printf ("%c", p[i]);
            }
        }
        printf ("\n");
    }
}
int key (int c, string v[])
{
    int m = strlen (v[1]);
    for (int i = 0; i <= m; i++)
    {
        if (v[1][0] == 0)
        {
            printf ("Usage: ./caesar key\n");
            return 1;
        }
        else if (v[1][i] >= 32 && v[1][i] < 48)
        {
            printf ("Usage: ./caesar key\n");
            return 1;
        }
        else if (v[1][i] >= 58 && v[1][i] <= 126)
        {
            printf ("Usage: ./caesar key\n");
            return 1;
        }
    }
    if (c != 2)
    {
        printf ("Usage: ./caesar key\n");
        return 1;
    }
    else
    {
        int r = atoi (v[1]);
        if (r < 1)
        {
            printf ("Usage: ./caesar key\n");
            return 1;
        }
        else
        {
            check = true;
            return r;
        }
    }
    return 0;
}

Solution

  • According to the C Standard (5.1.2.2.1 Program startup)

    2 If they are declared, the parameters to the main function shall obey the following constraints:

    — The value of argc shall be nonnegative.

    — argv[argc] shall be a null pointer....

    So when you run the program like

    ./caesar
    

    without specifying command line arguments then argc is equal to 1 and argv[argc] that is argv[1] is equal to NULL.

    Your program starts with calling the function key that in turn at once calls the standard C function strlen for the pointer v[1] that is equal to NULL.

    int key (int c, string v[])
    {
        int m = strlen (v[1]);
        //...
    

    This call invokes undefined behavior that results in a segmentation fault as you wrote.

    Before calling the function key you should at first check that argc is equal to 2, Or inside the function this if statement

    if (c != 2)
    {
        printf ("Usage: ./caesar key\n");
        return 1;
    }
    

    should b e placed before processing v[1].

    Also all these checks with numerous magic numbers

    for (int i = 0; i <= m; i++)
    {
        if (v[1][0] == 0)
        {
            printf ("Usage: ./caesar key\n");
            return 1;
        }
        else if (v[1][i] >= 32 && v[1][i] < 48)
        {
            printf ("Usage: ./caesar key\n");
            return 1;
        }
        else if (v[1][i] >= 58 && v[1][i] <= 126)
        {
            printf ("Usage: ./caesar key\n");
            return 1;
        }
    }
    

    are redundant. You could call at once the function strtol and check its execution whether it was successful..