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;
}```

CodePudding user response：

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.

CodePudding user response：

You've got two good answers already.

My suggestion is to consider both the user and those programmers who will revise your code. Inform the user of the command line problem and don't 'hide' the 'fail' process at the bottom... If the processing is to stop, show that at the top (and you can save one level of indent, too!)

int main( int argc, string argv[] )
{
    if( argc != 2 )
    {
        printf("Usage: ./caesar key [where key is all numeric]\n");
        return 1;
    }

    if( !only_digits( argv[1] ) )
    {
        printf( "Key must be an integer value\n");
        return 1;
    }
    int k = atoi( argv[1] );  // converts string n to k

    string plaintext = get_string( "plaintext:  " );

    printf( "ciphertext: " );
    for( int i = 0, l = strlen( plaintext ); i < l; i   )
    {
        printf( "%c", rotate( plaintext[i], k ) );
    }
    printf("\n");

    return 0;
}

CodePudding user response：

First of all, you dont have std::string type in C, only char*, TCHAR** etc.. like types. Then in if(only_digits(argv[1]) && argc == 2) even if argv[1] not defined, only_digits func is still called with not valid pointer => segfault

The size of the array pointed to by argv is at least argc 1, and the ONLY last element, argv[argc], is guaranteed to be a null pointer.