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

The code has undefined behavior because the initializer for the array planets has only 8 entries instead of 9, which is the value of NUM_PLANETS. When the command line string argument is not found among the 8 entries, you compare it to planets[8], which is an access beyond the end of the array and most probably an invalid pointer, causing the segmentation fault when is it dereferenced by strcmp.

You should add the missing entry "Neptune" in the initializer and you should compute the number of entries from the size of the array.

Here is a modified version:

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

int main(int argc, char *argv[]) {
    const char *planets[] = {
        "Mercury", "Venus", "Earth", "Mars", "Jupiter",
        "Saturn", "Uranus", "Neptune", "Pluto"
    };
    int num_planets = sizeof(planets) / sizeof(*planets);
    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]);
        }
    }
    return 0;
}

Alternatively, you could add a null pointer terminator in planets:

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

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

    for (int i = 1; i < argc; i++) {
        for (int j = 0;; j++) {
            if (!planets[j]) {
                printf("%s is not a planet\n", argv[i]);
                break;
            }
            if (strcmp(argv[i], planets[j]) == 0) {
                printf("%s is planet %d\n", argv[i], j + 1);
                break;
            }
        }
    }
    return 0;
}

---

Whether Pluto should or should not be present in the array is debatable. If you include Dwarf planets, other candidates should be present too: Eris, Haumea, Makemake, Gonggong, Quaoar, Sedna, Ceres, Orcus and Salacia...