Linked List value changes in gdb

Question

I have a C linked list that looks like this:

typedef struct Node {
    struct Node *child;
    void *value;
} Node;

typedef struct LinkedList {
    Node *head;
} LinkedList;

To test that everything is working properly, I have a main program that reads from a file, line by line, and stores each line in the following Node. Then, once the file reaches its end, I run through the linked list and print all of the lines.

However, when I test it, it only prints blank lines, except for the last line in the file, which gets printed normally. In addition, despite the fact that all the strings are malloc'd before they are stored in the nodes, I get a "pointer being free was not allocated error." I've gone through this pretty extensively in gdb and can't seem to figure out what I'm doing wrong. Perhaps somebody else can help me out here? Here's the rest of my code:

int main(int argc, char **argv) {
    if (argc>1) {
        FILE *mfile = fopen(argv[1], "r");
        if (mfile!=NULL) {
            char c;
            char *s = (char*) malloc(1);
            s[0] = '\0';
            LinkedList *lines = (LinkedList*) malloc(sizeof(LinkedList));
            while ((c=fgetc(mfile))!=EOF) {
                if (c=='\n') {
                    setNextLine(lines, s);
                    free(s);
                    s = (char*) malloc(1);
                    s[0] = '\0';
                }
                else s = append(s, c);
            }
            if (strlen(s)>0) {
                setNextLine(lines, s);
                free(s);
            }
            fclose(mfile);
            printList(lines);
            LLfree(lines);
        } else perror("Invalid filepath specified");
    } else perror("No input file specified");
    return 0;
}

void setNextLine(LinkedList *lines, char *line) {
    struct Node **root = &(lines->head);
    while (*root!=NULL) root = &((*root)->child);
    *root = (Node*) malloc(sizeof(Node));
    (*root)->child = NULL;
    (*root)->value = line;
}

char *append(char *s, char c) {
    int nl = strlen(s)+2;
    char *retval = (char*) malloc(nl);
    strcpy(retval, s);
    retval[nl-2] = c;
    retval[nl-1] = '\0';
    free(s);
    return retval;
}

void printList(LinkedList *lines) {
    Node *root = lines->head;
    while (root!=NULL) {
        char *s = (char*) root->value;
        printf("%s \n", s);
        root = root->child;
    }
}

void LLfree(LinkedList *list) {
    if (list->head!=NULL) NodeFree(list->head);
    free(list);
    return;
}

void NodeFree(Node *head) {
    if (head->child!=NULL) NodeFree(head->child);
    free(head->value);
    free(head);
    return;
}

Unrelated to your problem, but for the `append` function, you *do* know about [`realloc`](http://en.cppreference.com/w/c/memory/realloc)? — Some programmer dude, May 15 '14 at 06:07
Also, [there's no need to cast the return value of `malloc()` in C](http://stackoverflow.com/a/605858/28169), and it's better not to. — unwind, May 15 '14 at 06:07
And possibly related to your problem, you don't initialize the `LinkedList` structure you allocate, meaning `head` will have an indeterminate value (it will be seemingly random). Accessing this uninitialized member will lead to [*undefined behavior*](http://en.wikipedia.org/wiki/Undefined_behavior). — Some programmer dude, May 15 '14 at 06:10
"Invalid filepath specified" is not a particularly enlightening error message. Try `perror(argv[1])` — William Pursell, May 15 '14 at 06:12
You might like tu run you code using a memory checker like Valgrind (http://valgrind.org). — alk, May 15 '14 at 06:51
functions return values for a reason: `malloc` or `realloc` might return `NULL`. `malloc`/`realloc` can, and in some cases _will_ fail, check the return values, and _initialize_ memory you allocate to avoid problems — Elias Van Ootegem, May 15 '14 at 09:46

Mahonri Moriancumer · Answer 1 · 2014-05-15T07:42:09.710

It appears that there are several things that could be changed in the code. Perhaps the one that is most likely to help would be that memory improperly freed.

Change:

                setNextLine(lines, s);
                free(s);
                s = (char*) malloc(1);

to:

                setNextLine(lines, s);
//              free(s);
                s = (char*) malloc(1);

The pointer 's' is still pointing to what was just assigned to the previous node's 'value'. Hence, calling 'free(s)' is actually freeing the node's 'value'.

score 0 · Answer 2 · answered May 15 '14 at 08:07

0

Try doing this

void NodeFree(Node *head) 
    if (head->child!=NULL) 

        NodeFree(head->child);


    free(head->value);
    free(head->child);
    free(head);
    head->value = NULL;
    head->child = NULL;
    head = NULL;
    return;
}

answered May 15 '14 at 08:07

वरुण

1,237
3
18
50

M. Dahmani · Answer 3 · 2014-05-15T10:18:25.207

The setNextLine() function is appending the 's' poitner to the node value, and then that same pointer is getting freed after that call in the while loop.
That's why you'll get a double free fault when NodeFree() tries to free head->value. And the fact that you get the last line could be just because the address pointed by 's' for the last line (which got freed like all the previous lines) is still unused although its not allocated to your pointer anymore.
You should make a copy of the line pointed by 's' in setNextLine() so you can work with the 's' pointer for the rest of lines.

Linked List value changes in gdb

3 Answers3