How to dynamically insert strings in my linked list?

Question

My C program should read an input file and save it line for line in a linked list, then printing it. But when it's inserting the next line in the list, all elements in the list become the new element.

Input:

abc
def
ghi

Output:

ghi
ghi
ghi

I'm not sure but my guess is that it's not correctly allocating new memory for new elements in the list.

This is the code:

#include "listd.h"
#include <stdio.h>
void show(Value v) {
    printf(v);
    printf("\n");
}

int main( void ) {
    List lines = init();
    char a;
    while(a != EOF){
        a=getchar();
        char currentline[256] = " ";
        int i = 0;
        while(a != '\n' && a != EOF){
            currentline[i]=a;
            a = getchar();
            i++;
        }
        lines = insert(currentline, lines); //Here is probably the error
    }
    iterate(lines, show);
}

listd.h:

#include <stdlib.h>
#include <stdio.h>
#define DEFAULT 0

typedef enum {False ,True} Bool;
typedef char* Value;
typedef void (*VProc)(Value);
typedef struct _list {
    Value val;
    struct _list *next;
} Elem , *List;

List init() { return NULL; }
List insert(Value v, List l) {
    List e = malloc( sizeof (Elem));
    e->val = v;
    e->next = l;
    return e ;
}
void iterate(List l, VProc p) {
    for (;!empty(l);l=tail(l)) p(head(l));
    }

listd.h was given to us. But it was made for int, so I changed

typedef int Value;

to

typedef char* Value;

All data members val of nodes point to the same local array aktzeile. So what you do is what you get. You should dynamically allocate character arrays for data member val and copy strings into them. — Vlad from Moscow
You're going to have to duplicate that string or you're stomping the same memory location with each line. strdup() or strncpy() into a new buffer. — tadman
@Ben Take into account that this code snippet char a; while(a != EOF){ invokes undefined behavior because the variable a is not initialized and has indeterminate value. — Vlad from Moscow
@Vlad Yes, you are right. When I change aktzeile I change the output. Thanks for your help, I see if I can solve the problem now. — Ben
@Ben, just a tip, while not an error, the standard coding style for C avoids the use of camelCase or MixedCase variable names in favor of all lower-case while reserving upper-case names for use with macros and constants. It's style, so it up to you, but when you fail to follow it, it says a lot about your code before you even get to the details :) — David C. Rankin

sudipta sudipta · Accepted Answer · 2018-01-07T02:35:19

Issue1: In struct _list, val is a char pointer. When you do malloc( sizeof (Elem)), only memory for the two pointers(char*, struct_list*) is allocated.

Issue2: When e->val = v is executed it copies the char* v to e->val. It does not copy the string but only pointer. Here v is currentline. So it is obvious that e->val for all the elements will be the pointer to the last value which is stored in currentline. In this case, the last value is "ghi".

Please try below code and let me know:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#define DEFAULT 0

typedef enum {False ,True} Bool;
typedef char* Value;
typedef void (*VProc)(Value);
typedef struct _list {
    Value val;
    struct _list *next;
} Elem , *List;

List init() { return NULL; }
List insert(Value v, List l) {
    List e = malloc( sizeof (Elem));
    e->val = malloc(strlen(v) * sizeof(char));
    strcpy(e->val, v);
    e->next = l;
    return e ;
}

How to dynamically insert strings in my linked list?

2 Answers