1

I am creating a linked list in C with the syntax shown below

struct int_list
{
    int data;
    struct int_list *next;
    struct int_list *previous;
} int_list;

typedef struct
{
    size_t active_length;
    struct int_list *head;
    struct int_list *tail;
    struct int_list *current;
    bool init_status;
} int_data;

int init_llist(int_data *vec) {
    struct int_list *dat = malloc(sizeof(int_list));
    if (!dat) {
        fprintf(stderr, "Error in malloc\n");
        return - 1;
    }
    dat->previous = NULL;

    vec->head = dat;
    vec->tail = NULL;
    vec->current = dat;
    vec->active_length = 0;
    vec->init_status = true;
    return 1;
}

int push_llist(int_data *vec, int data, size_t index) {
    if (index < 0 || index > vec->active_length) {
        fprintf(stderr, "Index out of range\n");
        return -1;
    }
    struct int_list *dat = malloc(sizeof(int_list));
    if (!dat) {
        fprintf(stderr, "Error in malloc\n");
        return - 1;
    }

    if (index == 0 && vec->active_length > 0) {
        dat->previous = NULL;
        dat->next = vec->head;
        dat->data = data;

        (vec->head)->previous = dat;
        vec->head = dat;
        vec->active_length += 1;
    }
    else if (index == vec->active_length) {
        (vec->current)->data = data;
        (vec->current)->next = dat;

        dat->previous = (vec->current);

        vec->active_length += 1;
        vec->tail = dat;
        vec->current = dat;
    }
    else if (index < vec->active_length / 2) {
        struct int_list *current = vec->head;
        for (size_t i = 0; i < index; i++) {
            current = current->next;
        }
        dat->data = data;
        dat->next = current;
        dat->previous = current->previous;

        (current->previous)->next = dat;
        (current->next)->previous = dat;
        vec->active_length += 1;
    }
    else {
        struct int_list *current = vec->tail;
        for (size_t i = vec->active_length; i > index; i--) {
            current = current->previous;
        }
        dat->data = data;
        dat->data = data;
        dat->next = current;
        dat->previous = current->previous;

        (current->previous)->next = dat;
        (current->next)->previous = dat;
        vec->active_length += 1;
    }
    return 1;
}

void free_list(int_data *vec) {
    if (vec->active_length > 0) {
        struct int_list *tmp;
        struct int_list *head = vec->head;
        while (head->next != NULL) {
            tmp = head;
            head = tmp->next;
            free(tmp);
        }
        free(head);
    }
    else {
        struct int_list *head = vec->head;
        free(head);
    }
    //free(head);
}

int main(int argc, const char * argv[]) {
    int_data vec;
    init_llist(&vec);
    push_llist(&vec, 1, 0);
    free_list(&vec);
    return 0;
}

The implementation appears to work fine. However, when I run it using Valgrind it shows some issues that I do not understand. As I suspected, it does not show any memory leaks, but it is showing the following error when I run it with the following command valgrind -s --leak-check=full --track-origins=yes ./test

==3138== Memcheck, a memory error detector
==3138== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==3138== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==3138== Command: ./test
==3138== 
==3138== Conditional jump or move depends on uninitialised value(s)
==3138==    at 0x1093C8: free_list (main.c:125)
==3138==    by 0x109415: main (main.c:152)
==3138==  Uninitialised value was created by a heap allocation
==3138==    at 0x4841888: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3138==    by 0x1091FE: push_llist (main.c:56)
==3138==    by 0x10940D: main (main.c:142)
==3138== 
==3138== 
==3138== HEAP SUMMARY:
==3138==     in use at exit: 0 bytes in 0 blocks
==3138==   total heap usage: 2 allocs, 2 frees, 48 bytes allocated
==3138== 
==3138== All heap blocks were freed -- no leaks are possible
==3138== 
==3138== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
==3138== 
==3138== 1 errors in context 1 of 1:
==3138== Conditional jump or move depends on uninitialised value(s)
==3138==    at 0x1093C8: free_list (main.c:125)
==3138==    by 0x109415: main (main.c:152)
==3138==  Uninitialised value was created by a heap allocation
==3138==    at 0x4841888: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3138==    by 0x1091FE: push_llist (main.c:56)
==3138==    by 0x10940D: main (main.c:142)
==3138== 
==3138== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

It appears to take issue with how I allocated the int_list struct in the push_llist function. I cannot tell if this is just a bug in the Valgrind executable or if I have a legitimate issue to fix. Regardless, if this is coded poorly, I would appreciate any help to instruct my why this syntax is incorrect.

Craig Estey
  • 30,627
  • 4
  • 24
  • 48
Jon
  • 1,621
  • 5
  • 23
  • 46
  • Can't use just use C++, an std::vector and a class with a constructor that initializes everything? Less code and probably much more efficient. – Paul Floyd Nov 14 '22 at 16:09
  • Please post a complete [Minimal, Reproducible Example](https://stackoverflow.com/help/minimal-reproducible-example). Make sure your Valgrind report is accurate to the code you post (line numbers aligning correctly). The error implies that some member of the memory returned by `malloc` was used without initialization. You can try `calloc`, but that may just cause the bug to manifest in a different way. – Oka Nov 14 '22 at 17:35
  • In `init_llist`: Setting the `head` and `current` members (but not `tail`?) to a largely uninitialized, dummy node is ... strange. This partial initialization may be having knock-on effects. – Oka Nov 14 '22 at 17:50
  • @PaulFloyd Unfortunately I am limited to using C for this problem, not C++ – Jon Nov 15 '22 at 02:00
  • @Oka as you have correctly summarized I have taken some aspects out of the example that did not highlight the point and will re-post. However, main.c:125 refers to the line in the push_list function that malloc's a struct implementation. – Jon Nov 15 '22 at 02:03

1 Answers1

1

It turns out that my problem was that I was not adequately updating the tail and head values in the int_list struct. In addition, while this was not responsible for the problem that led me to post the question, this would also cause the code to overwrite values in the struct. The answer posted below solves the problem that led to this post.

struct int_list
{
    int data;
    struct int_list *next;
    struct int_list *previous;
} int_list;

typedef struct
{
    size_t active_length;
    struct int_list *head;
    struct int_list *tail;
    struct int_list *current;
    bool init_status;
} int_data;

int init_llist(int_data *vec) {
    struct int_list *dat = malloc(sizeof(int_list));
    if (!dat) {
        fprintf(stderr, "Error in malloc\n");
        return - 1;
    }
    dat->previous = NULL;

    vec->head = dat;
    vec->tail = dat;
    vec->current = dat;
    vec->active_length = 0;
    vec->init_status = true;
    return 1;
}

int push_llist(int_data *vec, int data, size_t index) {
    if (index < 0 || index > vec->active_length) {
        fprintf(stderr, "Index out of range\n");
        return -1;
    }
    struct int_list *dat = malloc(sizeof(int_list));
    if (!dat) {
        fprintf(stderr, "Error in malloc\n");
        return - 1;
    }

    if (index == 0 && vec->active_length > 0) {
        dat->previous = NULL;
        dat->next = vec->head;
        dat->data = data;

        (vec->head)->previous = dat;
        vec->head = dat;
        vec->active_length += 1;
    }
    else if (index == vec->active_length) {
        (vec->current)->data = data;
        (vec->current)->next = dat;

        dat->previous = (vec->current);
        dat->next = NULL;

        vec->active_length += 1;
        vec->tail = dat;
        vec->current = dat;
    }
    else if (index < vec->active_length / 2) {
        struct int_list *current = vec->head;
        struct int_list *previous;
        for (size_t i = 0; i < index; i++) {
            previous = current;
            current = current->next;
        }
        dat->data = data;
        dat->next = current;
        dat->previous = previous;

        previous->next = dat;
        current->previous = dat;
        vec->active_length += 1;
    }
    else {
        struct int_list *current = vec->tail;
        struct int_list *next;
        for (size_t i = vec->active_length; i > index; i--) {
            next = current;
            current = current->previous;
        }
        dat->data = data;
        dat->next = next;
        dat->previous = current;

        next->previous = dat;
        current->next = dat;

        vec->active_length += 1;
    }
    return 1;
}

void print_list(int_data *vec) {
    struct int_list *dat = vec->head;
    int i = vec->active_length-1;
    while (i >= 0) {
        printf("%d\n", dat->data);
        dat = dat->next;
        i--;
    }
}

void free_list(int_data *vec) {
    if (vec->active_length > 0) {
        struct int_list *tmp;
        struct int_list *head = vec->head;
        while (head->next != NULL) {
            tmp = head;
            head = tmp->next;
            free(tmp);
        }
        free(head);
    }
    else {
        struct int_list *head = vec->head;
        free(head);
    }
}

int insert_vector(int_data *vec, int *arr, size_t length, size_t index) {
    int test;
    for (size_t i = 0; i < length; i++) {
        test = push_llist(vec, arr[i], index);
        if (test == -1) return -1;
        index++;
    }
    return 1;
}
// Begin code
int main(int argc, const char * argv[]) {
    int_data vec;
    init_llist(&vec);
    push_llist(&vec, 1, 0);
    push_llist(&vec, 2, 1);
    push_llist(&vec, 3, 2);
    push_llist(&vec, 4, 3);
    push_llist(&vec, 5, 4);
    push_llist(&vec, 6, 5);
    push_llist(&vec, 555, 6);
    int a[4] = {12, 13, 14, 15};
    insert_vector(&vec, a, 4, 3);
    printf("\n");
    print_list(&vec);

    free_list(&vec);
    return 0;
}
Jon
  • 1,621
  • 5
  • 23
  • 46