Why am I getting Segmentation fault (core dumped) message?

Question

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?

If you don't give any arguments to the command, `strlen(key)` fails because `argv[1]` is a null pointer. — Barmar, Sep 05 '22 at 22:00
Thank you! That is exactly what was going wrong. I was able to fix it. I appreciate the advice Barmar. — Jack Finnegan, Sep 05 '22 at 22:05
Depending on the age of the compiler, as written above, not all "paths" lead to a `return` statement... This would give errors (if the compiler is "older" and didn't "fill-in" the absent `return 0;` before the closing brace of `main()`. — Fe2O3, Sep 05 '22 at 22:30
Does this answer your question? [What is a segmentation fault?](https://stackoverflow.com/questions/2346806/what-is-a-segmentation-fault) — PiRocks, Sep 05 '22 at 23:52

Fe2O3 · Answer 1 · 2022-09-05T23:49:06.547

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

Minor suggestion: I would move `string key = argv[1];` _before_ the check on the argument's length as you can then write: `strlen(key) != 26` which is more expressive. — Chris, Sep 05 '22 at 22:43
@Chris Yes, equally valid. Or: `#define KEY argv[1]`... To my mind, it is not a `key` until it has been shown to be a `key`... The other requirement of "all unique monocase alphabetic" should be checked, imo, before argv[1] is called a 'key'... Your suggestion is perfectly fine, too. `:-)` — Fe2O3, Sep 05 '22 at 22:50
So `string valid_key = argv[1];` at that position is definitely out. ;) — Chris, Sep 05 '22 at 22:50
@Chris That would be terrific (once all the hurdles are passed) `:)` Just 'tweaked' the error message to be more helpful to the user. `:-)` — Fe2O3, Sep 05 '22 at 22:56
@Chris Justified my rationale (a bit) with an alternative bit of code. `:-)` — Fe2O3, Sep 05 '22 at 23:26
@RafaelplayerxdYT The two have exactly the same effect in this example, but I would favour `exit()` both as it **stands out** as something bad has happened, and, if the code block is moved to a subordinate function, `return` will not suffice to terminate execution. In this example, using `return` from `main()` would be ample, yes. `:-)` — Fe2O3, Sep 05 '22 at 23:47

user16217248 · Answer 2 · 2022-09-05T22:55:50.027

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

In some circles, SP (0x20) is considered an "ASCII character"... Why can't I, the user, enter SP as one of the 26 characters??? :-) — Fe2O3, Sep 05 '22 at 23:05
@Fe2O3 But you can. You can put it in quotes, or put a backslash (escape character) in front of it. — user16217248, Sep 05 '22 at 23:07

Why am I getting Segmentation fault (core dumped) message?

2 Answers2

Correct Program: