Segmentation Fault while running following C program

Question

This C program takes planet names as arguments and prints if they are planet or not. The good case is working

./planets Venus Mercury

But if I add bad case I am getting Segmentation Fault.

./planets Venus Mercury mercury

What can be the reason for this? Thanks in advance.

#include <stdio.h>
#include <string.h>

#define NUM_PLANETS 9

int main(int argc, char* argv[]) {
    char *planets[] = { "Mercury", "Venus", "Earth", "Mars", "Jupiter", "Saturn", "Uranus", "Pluto" };
    int i, j;
    for (i = 1; i < argc; i++) {
        for (j = 0; j < NUM_PLANETS; j++) {
            if (strcmp(argv[i], planets[j]) == 0) {
                printf("%s is planet %d\n", argv[i], j + 1);
                break;
            }
        }
        if (j == NUM_PLANETS) {
            printf("%s is not a planet\n", argv[i]);
        }
    }
}

Answer 1

You declared an array of 8 elements

char *planets[] = { "Mercury", "Venus", "Earth", "Mars", "Jupiter", "Saturn", "Uranus", "Pluto" };

but in the condition of the inner for loop

for (j = 0; j < NUM_PLANETS; j++) {

you are using the macro NUM_PLANETS that is set to 9

#define NUM_PLANETS 9

As a result within the for loop there can be an access to memory outside the array that results in undefined behavior.

Using the macro makes your program error-prone.

Instead you could within main to determine the real number of elements in the array the following way

char *planets[] = { "Mercury", "Venus", "Earth", "Mars", "Jupiter", "Saturn", "Uranus", "Pluto" };
const size_t NUM_PLANETS = sizeof( planets ) / sizeof( *planets );

In this case even if the array will be enlarged supplying new string literals used as initializers your program will be correct and you will need to change nothing.

The second remark is that using the statement break within the inner for loop makes your program less readable. It is just a bad style of programming.

And you should declare variables in minimum scopes where they are used.

Instead you could write the program for example the following way

#include <stdio.h>
#include <string.h>

int main(int argc, char* argv[]) 
{
    const char *planets[] = 
    { 
        "Mercury", "Venus", "Earth", "Mars", "Jupiter", "Saturn", "Uranus", "Pluto" 
    };
    const size_t NUM_PLANETS = sizeof( planets ) / sizeof( *planets );

    for ( int i = 1; i < argc; i++ ) 
    {
        size_t j = 0;

        while ( j < NUM_PLANETS && strcmp( argv[i], planets[j] ) != 0 ) ++j;

        if ( j != NUM_PLANETS ) 
        {
            printf( "%s is planet %zu\n", argv[i], j + 1 );
        }
        else
        {
            printf( "%s is not a planet\n", argv[i] );
        }
    }
}

Pay attention to the qualifier const within the type specifiers in the array declaration. Though in C string literals have types of non-constant character arrays (by historical reasons) nevertheless any attempt to change a string literal results in undefined behavior. So it is better to define such arrays as in the program with the qualifier const. Moreover in C++ string literals have types of constant character arrays. So you wll be even able to compile your program using a C++ compiler.