strcmp() Segmentation Fault in C

Question

When I call this function, my code always breaks in the strcmp and returns a Segmentation Error with no more information provided.

stop_t *getStop(char *name) {
    node_t *current = stop_list_head;
    stop_t *stop;

    while (current != NULL) {
        stop = current->stop;
        if (stop != NULL) {
            if (stop->name != NULL) {
                if (strcmp(stop->name, name) == 0) {
                    return stop;
                }
            }
        }
        current = current->next;
    }
    return NULL;
}

When I Inserted a printf("%s", stop->name); it returned the same Segmentation Fault but on the printf.

How can I fix this?

Minimal Reproducible Example:

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

#define MAX_LENGTH_STOP 50

typedef struct {
    int routeCounter;
    double latitude;
    double longitude;
    char name[MAX_LENGTH_STOP + 1];
} stop_t;

typedef struct node {
    stop_t *stop;
    struct node *next;
} node_t;

void printStopList();

node_t *stop_list_head = NULL;

int main() {
    stop_t *stopPtr = NULL;
    stop_t stop;
    char name[MAX_LENGTH_STOP + 1];

    /* Input Example: Praca de Espanha ; */
    fgets(name, BUFSIZ, stdin);

    stopPtr = getStop(name);
    /* Create new stop if it doesn't exist already, else print error.*/
    if (stopPtr == NULL) {
        generateStop(name);
    } else {
        printf("<Error 01>: Stop already exists.\n");
    }
}

/* Determines if the stop already exists based on the name */
stop_t *getStop(char *name) {
    node_t *current = stop_list_head;
    stop_t *stop;

    while (current != NULL) {
        stop = current->stop;
        if (stop != NULL) {
            if (stop->name != NULL) {
                if (strcmp(stop->name, name) == 0) {
                    return stop;
                }
            }
        }
        current = current->next;
    }
    return NULL;
}

/* Generates a stop instance and adds it to the global linked list*/
void generateStop(char name[]) {
    stop_t *stop = NULL;

    stop = (stop_t *)malloc(sizeof(stop_t));

    strcpy(stop->name, name);
    stop->routeCounter = 0;
    addStopToList(stop);
    free(stop);
}

/* Adds created stops to a global linked list (stop_list_head)*/
void addStopToList(stop_t *stop) {

    node_t *new_node = (node_t *)malloc(sizeof(node_t));
    node_t *current;

    new_node->stop = stop;
    new_node->next = NULL;

    if (stop_list_head == NULL) {
        stop_list_head = new_node;
    } else {
        current = stop_list_head;
        while (current->next != NULL) {
            current = current->next;
        }
        current->next = new_node;
    }

    free(new_node);
}

Answer 1

There are multiple problems:

• you free the newly allocated structures in the functions supposed to allocate them. This causes undefined behavior when they are later read, eg: when strcmp accesses stop->name

• you read name with fgets passing an invalid size BUFSIZ much larger than MAX_LENGTH_STOP + 1. This may cause a buffer overflow

• you do not test the return value of fgets(), causing invalid behavior if redirected from an empty file.

• fgets() leaves the trailing newline at the end of the name array: it will be stored into the stop list.

• strcpy(stop->name, name); may cause a buffer overflow if the name argument is longer than the name field in the stop_t structure.

• the minimal example does not actually cause a segmentation fault where you report it because the stop list is empty.

Here is a modified version:

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

#define MAX_LENGTH_STOP 50

typedef struct stop {
    int routeCounter;
    double latitude;
    double longitude;
    char name[MAX_LENGTH_STOP + 1];
} stop_t;

typedef struct node {
    stop_t *stop;
    struct node *next;
} node_t;

node_t *stop_list_head;

/* Determines if the stop already exists based on the name */
stop_t *getStop(const char *name) {
    node_t *current = stop_list_head;

    while (current != NULL) {
        stop_t *stop = current->stop;
        if (stop != NULL && strcmp(stop->name, name) == 0) {
            return stop;
        }
        current = current->next;
    }
    return NULL;
}

/* Adds created stops to a global linked list (stop_list_head)*/
void addStopToList(stop_t *stop) {
    node_t *new_node = (node_t *)malloc(sizeof(node_t));
    if (new_node == NULL) {
        perror("cannot allocate node_t");
        exit(1);
    }

    new_node->stop = stop;
    new_node->next = NULL;

    if (stop_list_head == NULL) {
        stop_list_head = new_node;
    } else {
        node_t *current = stop_list_head;
        while (current->next != NULL) {
            current = current->next;
        }
        current->next = new_node;
    }
}

/* Generates a stop instance and adds it to the global linked list*/
void generateStop(const char *name) {
    stop_t *stop = (stop_t *)calloc(sizeof(stop_t), 1);
    if (stop == NULL) {
        perror("cannot allocate stop_t");
        exit(1);
    }
    strncat(stop->name, name, sizeof(stop->name) - 1);
    addStopToList(stop);
}

int main(void) {
    for (;;) {
        char name[MAX_LENGTH_STOP + 2];

        /* Input Example: Praça de Espanha ; */
        if (!fgets(name, sizeof name, stdin))
            return 1;
        /* strip the trailing newline if present */
        name[strcspn(name, "\n")] = '\0';
        /* stop on an empty line */
        if (*name == '\0')
            break;
        /* Create new stop if it doesn't exist already, else print error.*/
        if (getStop(name)) {
            printf("<Error 01>: Stop already exists: %s\n", name);
        } else {
            generateStop(name);
        }
    }
    return 0;
}