Search code examples
csegmentation-faultcomputer-sciencecs50

CS50 - Problem Set 2 - encountering "segmentation fault (core dumped)"


My program seems to be working fine and it does as intended, it takes command line arguments and "rotates" the inputted string from the prompt depending on the inputted command line argument. However, if I run my code without any arguments like: ./caesar it doesn't work, it says "segmentation fault (core dumped)" but if i run it like this ./caesar 1 or any other number, it works as intended.

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

int only_digits(string n);
char rotate(char i, int n);

int main(int argc, string argv[])
{


    if(only_digits(argv[1]) && argc == 2) {
        string plaintext = get_string("plaintext:  ");
        printf("ciphertext: ");
        int k = atoi(argv[1]);  // converts string n to k

        for (int i = 0, l = strlen(plaintext); i < l; i++)
        {
            printf("%c", rotate(plaintext[i], k));

        }

        printf("\n");
    }

    else
    {
        printf("Usage: ./caesar key\n");
        return 1;
    }
}



int only_digits(string n) // function that returns 1 or 0 depending if given string is only digits
{
    int state;
    for(int i = 0, l = strlen(n); i < l; i++)
    {
        if (isdigit(n[i]))  // checks if characters in string is a digit from 1 to 9
        {
            state = 1;
        }
        else
        {
            state = 0;
            break;
        }
    }
    return state;
}

char rotate(char c, int n)
{
    char rotated_char = c;
    if (isupper(c))
    {
        int a_index = c - 'A';
        int c_cipher = (a_index + n) % 26;
        rotated_char = c_cipher + 'A';
    }
    else if (islower(c))
    {
        int a_index = c - 'a';
        int c_cipher = (a_index + n) % 26;
        rotated_char = c_cipher + 'a';
    }
    return rotated_char;
}```

Solution

  • The quickest and easiest fix is to replace:

    if(only_digits(argv[1]) && argc == 2)
    

    With:

    if(argc == 2 && only_digits(argv[1]))
    

    Just swap the two sub-expressions around. When argc is 1 (ie, no arguments), argv[1] is NULL, and only_digits() cannot handle that - strlen(NULL) is incorrect.

    However, if we do argc == 2 first, short-circuit evaluation rules say that since FALSE && anything is always FALSE, we don't need to bother evaluating only_digits(), so the code is never called and thus the crash is avoided.