CODEWITHSUNDEEP
clistlinked-listsegmentation-fault
ViscloDev
ViscloDev

Reputation: 19

segmentation fault when trying to display a list

I'm trying to display a list in C but when I do it I'm getting a segfault. Valgrind is telling me that it's because I'm trying to reach uninitialized values, can you help me?

Function to display the list (segfault at line 7):

void display_list(struct list *list)
{
    struct list *tmp;

    tmp = list;
    my_printf("liste =");    
    while (tmp != NULL) {
        my_printf(" %d", tmp->number);
        tmp = tmp->next;
    }
    my_printf("\n");
    free(tmp);
}

Structure of the list:

struct list
{
    int number;
    struct list_a *next;
};

main function:

int main(int argc, char **argv)
{
    struct list *list_a;
    struct list *list_b;
    for (int i = 1; i < argc; i++) {
        add_item(&list_a, my_getnbr(argv[i]));
    }
    display_list(list_a);
    free(list_a);
    free(list_b);
    return (0);
}

int add_item(struct list **list, int item)
{
    struct list *element = malloc (sizeof(*element));

    if (element == NULL)
        return (EXIT_FAILURE);
    element->number = item;
    element->next = *list;
    *list = element;
    return (0);
}

int my_getnbr(char const *str)
{
    int i = 0;
    int nbr = 0;

    if (str[0] == '-' && str[1] != '\0')
        i++;
    for (i; str[i] != '\0'; i++) {
        if (str[i] < '0' || str[i] > '9')
            write(2, "there must be only numbers in the string", 40);
        nbr = nbr + str[i] - 48;
        nbr = nbr * 10;
    }
    nbr = nbr / 10;
    if (str[0] == '-')
        return (-1 * nbr);
    else
        return (nbr);
}

Upvotes: 0

Views: 62

Answers (1)

dbush
dbush

Reputation: 223689

In your main function, you're calling free on list_b, but it is uninitialized meaning its valid is indeterminate. Attempting to call free on such a pointer invokes undefined behavior which causes a crash.

Get rid of that call to free, and in fact remove list_b entirely as it isn't being used.

Also, calling free at the end of display_list doesn't make sense because you don't need to clean up any memory here. You get away with it because the value of tmp is NULL at this point, and calling free on a NULL pointer is a no-op.

You also have list_a uninitialized. This results in your list having an invalid pointer at the end. You should initialize it to NULL.

Upvotes: 1

Related Questions