-1

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;
}```
opti
  • 1
  • 2
  • 2
    Have you tried running your code line-by-line in a debugger while monitoring the values of all variables, in order to determine in which line your program stops behaving as intended? If you did not try this, then you may want to read this: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/q/25385173/12149471) Note that CS50 has its own debugger, called [debug50](https://video.cs50.io/v_luodP_mfE?screen=J0ND72qsI9U&start=1688&end=2012). – Andreas Wenzel Aug 19 '22 at 01:41

2 Answers2

3

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.

Ken Y-N
  • 14,644
  • 21
  • 71
  • 114
3

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;
}
Fe2O3
  • 6,077
  • 2
  • 4
  • 20