int main(int argc, string argv[])
{
    int count = argc;
    string key = argv[1];
    int keylength = strlen(key);

    if (keylength < 26)
    {
        printf("Key must contain 26 characters\n");
    }
    else if (argc < 1)
    {
        printf("Usage: ./substitution key\n");
    }
    else
    {
        return 0;
    }
}

Every time I call the program with "./substitution" and no other command line arguments, it gives me the message "Segmentation fault (core dumped)" when I just wanted it to print back the line describing what the user should be putting in. Does anyone understand why this is happening?

CodePudding user response：

int main(int argc, string argv[])
{
    // int count = argc; // unnecessary duplication
    string key = argv[1]; // Too optimistic
    int keylength = strlen(key); // undefined behaviour when no "argument" supplied

    /* already "off the rails */
    /* ... */
}

Perform operations in a reasonable sequence

#include <stdio.h>
#include <stdlib.h> // for 'exit()'
#include <string.h>
#include "cs50.h" // EDIT. Required for "string" datatype

int main(int argc, string argv[])
{
    if( argc != 2 )
    {
        printf("Usage: ./substitution key\n");
        exit( EXIT_FAILURE ); // <<** called "early termination"
    }
    // When "flow" has been redirected with
    // exit(), return, break, continue or goto
    // PLEASE do not write a pointless "else" clause.
    // Any code that follows IS the "else" condition.

    if( strlen( argv[1] ) != 26 ) // ONLY 26 is acceptable
    {
        printf("Key must contain ONLY 26 unique alphabetic characters\n");
        exit( EXIT_FAILURE );
    }

    string key = argv[1];

    /* more code */

    return 0;
}

There could be less code by combining two checks (in the correct sequence!), providing the erring user with useful information.

int main(int argc, string argv[])
{
    if( argc != 2 || strlen( argv[1] ) != 26 )
    {
        printf("Usage: ./substitution key\n");
        printf("Key must contain ONLY 26 unique alphabetic characters\n");
        exit( EXIT_FAILURE ); // <<** called "early termination"
    }

    /* more validation of parameter at argv[1] */

    string key = argv[1]; // now acceptable. use alias.

    /* more code */

    return 0;
}

Since there is more than one "check" to perform before getting into the nitty-gritty of processing, perhaps all the validation checks could be done in a function that returns go/no-go:

int invalidKey( string candidate ) {
    if( strlen( candidate ) != 26 )
        return 1; // bad

    /* more validation of "key" returning 1 if objectionable */

    return 0; // good; passed all the tests
}

int main(int argc, string argv[])
{
    if( argc != 2 || invalidKey( argv[1] ) )
    {
        printf("Usage: ./substitution key\n");
        printf("Key must contain ONLY 26 unique alphabetic characters\n");
        exit( EXIT_FAILURE ); // <<** called "early termination"
    }

    string key = argv[1];

    /* more code */

    return 0;
}

CodePudding user response：

Every time I call the program with "./substitution" and no other command line arguments, it gives me the message "Segmentation fault (core dumped)"

If there are no other arguments, then argv[1] yields NULL, which is then assigned to key which is passed to strlen(), and passing a NULL value to strlen() invokes undefined behavior.

Also argc will never be less than one because the command executing the program will always be the first argument. You should check if argc < 2 before attempting to access the arguments.

Correct Program:

int main(int argc, string argv[]) {
    if (argc < 2) {
        printf("Usage: ./substitution key\n");
        return -1;
    }
    string key = argv[1];
    int keylength = strlen(key);
    if (keylength != 26) {
        printf("Key must contain exactly 26 characters\n");
        return -1;
    }
    // ...
    return 0;
}