Memory error when trying to implement my own list

Question

I'm trying to implement my own linked-list like structure in C, however I'm failing even with easiest part of just adding new elements to the list. Adding one element to the list and printing it works fine, adding another and printing the one doesn't. After executing it the console simply outputs "memory error" - that's it. I'm pretty sure I've messed up with the iteration of the current-pointer in my insert_list function, but I can't find where.

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

#define MAXLEN 80

typedef struct list
{
    struct list *next;
    char value[MAXLEN];
} list;

void insert_list(list *lis, const char string[MAXLEN])
{
    list *current = lis;
    while(current->next)
    {
        current++;
    }
    current->next = malloc(sizeof(lis));
    strcpy(current->next->value, string);
}

int main(void)
{
    list lis =
    {
        NULL,
        "Hello1"
    };
    insert_list(&lis, "Hello2");
    insert_list(&lis, "Hello3");
    /* This still works */
    printf("%s %s", lis.value, lis.next->value);
    /* This doesn't */
    printf("%s", lis.next->next->value);

    return 0;
}

When you say "gcc outputs...", what do you actually mean? What do you run to get that error? — Mad Physicist, Feb 11 '16 at 16:22
Are you *quite sure* `while(current->next) { current++; }` in `insert_list` is what you want, to get the next element of a *linked list*? — EOF, Feb 11 '16 at 16:23
"gcc simply outputs "memory error" " -- can you show us the GCC invocation and exact output? Are you sure GCC is showing this error? — Jonah Graham, Feb 11 '16 at 16:23
I am asking because I do not think that gcc can output such a thing. — Mad Physicist, Feb 11 '16 at 16:23
My bad. I compiled it with gcc using gcc -g -ansi -Wall -pedantic list.c -o list - then using ./list to execute it which gave me the error. — Philip Feldmann, Feb 11 '16 at 16:26
Mind telling me what it does, @MartinJames? Not everyone is programming C for years, I just started recently in university and we are not using a debugger so far. — Philip Feldmann, Feb 11 '16 at 16:28
@PhilipFeldmann you need to learn how to use a debugger. Never mind what your prof/TA say or do, whatever timtable they are on, learn to use gdb, or whatever NOW. That's what good students do - they learn stuff by themselves. — Martin James, Feb 11 '16 at 17:07
sizeof(lis) returns the size of a pointer on your system, not the size of 'list'. — Martin James, Feb 11 '16 at 17:09
It sounds like you may need to learn how to use a debugger to step through your code. With a good debugger, you can execute your program line by line and see where it is deviating from what you expect. This is an essential tool if you are going to do any programming. Further reading: **[How to debug small programs](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/)** — NathanOliver, Feb 11 '16 at 20:26

score 3 · Accepted Answer · edited May 23 '17 at 11:45

current->next->next is never initialized in insert_list, so the second time you call insert_list it will probably iterate forever until a memory error.

void insert_list(list *lis, const char string[MAXLEN])
{
    list *current = lis;
    while(current->next)
    {
        current++;
    }
    current->next = malloc(sizeof(lis));
    current->next->next = NULL;
    strcpy(current->next->value, string);
}

Edit

Actually there are lots of other errors, not just the init:

advancing the list, should probably be current = current->next, not current++
malloc should be sizeof list not lis, first is the entry in a list size, but sizeof(lis) will be the size of a pointer on your machine
const char string[MAXLEN] as an argument does not mean what you think it means ;-) See Difference between passing array and array pointer into function in C

I knew I messed up somewhere in this area, I didn't expect it to be that bad. Should have noticed myself, thank you very much! — Philip Feldmann, Feb 11 '16 at 16:30
Don't worry, keep going with the learning. I am pleased that you are learning C in university as too many people skip over it and don't get appreciation for the reality of lower levels. — Jonah Graham, Feb 11 '16 at 16:31

Memory error when trying to implement my own list

1 Answers1

Edit