Why can I not add to a linked list correctly when using a function?

Question

I am trying to a create a linked list. When I add to the list in the same function that I create the object in, it works.

Definitions:

typedef struct student {
    int num;
    char* name;
} student;

typedef struct container {
    student* data;
    struct container* next;
} container ;

All the objects I use are initialized like this:

student stu1;
stu1.num = 6;
stu1.name = "grefagf";

front = createContainer(&stu1);
back = front;

student stu2;
stu2.num = 3;
stu2.name = "dsghjyreawre";

student stu3;
stu3.num = 4;
stu3.name = "dsghhjrant";

student stu4;
stu4.num = 213;
stu4.name = "fdsafgrw";

When I add these elements to a list in the main function like this:

container* tmp;

tmp = createContainer(&stu2);
back->next = tmp;
back = tmp;

tmp = createContainer(&stu3);
back->next = tmp;
back = tmp;

tmp = createContainer(&stu4);
back->next = tmp;
back = tmp;

it works properly, outputting this:

1:  6       grefagf
2:  3       dsghjyreawre
3:  4       dsghhjrant
4:  213     fdsafgrw

when I print it using another function I made.

But if I create a function called add() and pass stu2, stu3..., like so:

int add(student to_add) {

    container* tmp;
    tmp = createContainer(&to_add);
    printf("added: (%d, %s)\n", tmp->data->num, tmp->data->name);

    back->next = tmp;
    back = tmp;

    return 1;
}

then do this in the main function:

add(stu2);
add(stu3);
add(stu4);

it outputs this:

1:  6       grefagf
2:  41096808        fdsafgrw
3:  41096808        fdsafgrw
4:  41096808        fdsafgrw

Heres the source in case you need it:

Non-function example: https://pastebin.com/ZLqTzp4t

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

typedef struct student {
    int num;
    char* name;
} student;

typedef struct container {
    student* data;
    struct container* next;
} container ;

container* back;
container* front;

container* createContainer(student* data) {

    container* tmp = malloc(sizeof(container));

    tmp->data = data;
    tmp->next = NULL;

    return tmp;
}

void printList(container* front) {

    container* tmp = front;

    int i;
    i=0;
    while (tmp != NULL) {
        i++;
        printf("%d:\t%d\t\t%s\n", i, tmp->data->num, tmp->data->name);
        tmp = tmp->next;
    }
}

int main(void) {

    student stu1;
    stu1.num = 6;
    stu1.name = "grefagf";

    front = createContainer(&stu1);
    back = front;

    student stu2;
    stu2.num = 3;
    stu2.name = "dsghjyreawre";

    student stu3;
    stu3.num = 4;
    stu3.name = "dsghhjrant";

    student stu4;
    stu4.num = 213;
    stu4.name = "fdsafgrw";

    container* tmp;

    tmp = createContainer(&stu2);
    back->next = tmp;
    back = tmp;

    tmp = createContainer(&stu3);
    back->next = tmp;
    back = tmp;

    tmp = createContainer(&stu4);
    back->next = tmp;
    back = tmp;

    printf("front\n");
    printList(front);
    printf("\ntop\n");
    printList(back);

    return EXIT_SUCCESS;
}

Function example: https://pastebin.com/TyQY4j5k

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

typedef struct student {
    int num;
    char* name;
} student;

typedef struct container {
    student* data;
    struct container* next;
} container ;

container* back;
container* front;

container* createContainer(student* data) {

    container* tmp = malloc(sizeof(container));

    tmp->data = data;
    tmp->next = NULL;

    return tmp;
}

int add(student to_add) {

    container* tmp;
    tmp = createContainer(&to_add);
    printf("added: (%d, %s)\n", tmp->data->num, tmp->data->name);

    back->next = tmp;
    back = tmp;

    return 1;
}

void printList(container* front) {

    container* tmp = front;

    int i;
    i=0;
    while (tmp != NULL) {
        i++;
        printf("%d:\t%d\t\t%s\n", i, tmp->data->num, tmp->data->name);
        tmp = tmp->next;
    }
}

int main(void) {

    student stu1;
    stu1.num = 6;
    stu1.name = "grefagf";

    front = createContainer(&stu1);
    back = front;

    student stu2;
    stu2.num = 3;
    stu2.name = "dsghjyreawre";

    student stu3;
    stu3.num = 4;
    stu3.name = "dsghhjrant";

    student stu4;
    stu4.num = 213;
    stu4.name = "fdsafgrw";

    add(stu2);
    add(stu3);
    add(stu4);

    printf("front\n");
    printList(front);
    printf("\ntop\n");
    printList(back);

    return EXIT_SUCCESS;
}

Please don't link to your code, but produce a proper minimal reproducible example in the question itself. It'd seem that you're passing a struct by value to add(), but you've not included the definition of student. — Ilja Everilä
– Ilja Everilä, Commented Feb 11, 2018 at 19:24
@IljaEverilä I wanted to include both an example with the function and without and I figured it would be easier to link to pastebin where it's easy to copy/paste than to write it here. And I added the function definitions. — Alexander
– Alexander, Commented Feb 11, 2018 at 19:34
Please rethink; we shouldn't need to go offsite to see the code you're asking about. Read up on creating an MCVE (minimal reproducible example). Minimal is important; so is complete. Note to that the code student stu1; stu1.num = 6; stu1.name = "grefagf"; is not an initializer for stu1; it assigns values to it. It would be initialized with student stu1 = { 6, "grefagf" }; or student stu1 = { .num = 6, .name = "grefagf" }; — where the second uses designated initializers. (IOW: "initializer" is a technical term in C and applies to 'assigned when defined'.) — Jonathan Leffler
– Jonathan Leffler, Commented Feb 11, 2018 at 20:04
why do not you give the address of stu2 to add function such as add(&stu2) and revise your functions as int add(student * to_add) — RedArrow
– RedArrow, Commented Feb 11, 2018 at 20:09
it seems about casting because your num variable seem take address values now i will try your code and return to you — RedArrow
– RedArrow, Commented Feb 11, 2018 at 20:14

Jonathan Leffler · Accepted Answer · 2018-02-11 20:30:00Z

3

Here's a more or less minimal version of your code with the add function. It runs at 60 lines compared to 83 for your original.

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

typedef struct student
{
    int num;
    char *name;
} student;

typedef struct container
{
    student *data;
    struct container *next;
} container;

static container *back;
static container *front;

static container *createContainer(student *data)
{
    container *tmp = malloc(sizeof(container));
    tmp->data = data;
    tmp->next = NULL;
    return tmp;
}

static void add(student to_add)
{
    container *tmp = createContainer(&to_add);
    printf("added: (%d, %s)\n", tmp->data->num, tmp->data->name);
    back->next = tmp;
    back = tmp;
}

static void printList(container *item)
{
    for (int i = 0; item != NULL; item = item->next)
        printf("%d:\t%d\t\t%s\n", ++i, item->data->num, item->data->name);
}

int main(void)
{
    student stu1 = { 6, "grefagf" };
    student stu2 = { 3, "dsghjyreawre" };
    student stu3 = { 4, "dsghhjrant" };
    student stu4 = { 213, "fdsafgrw" };

    front = createContainer(&stu1);
    back = front;
    add(stu2);
    add(stu3);
    add(stu4);

    printf("front\n");
    printList(front);
    printf("\nback\n");
    printList(back);

    return EXIT_SUCCESS;
}

On my Mac, it produced:

added: (3, dsghjyreawre)
added: (4, dsghhjrant)
added: (213, fdsafgrw)
front
1:  6       grefagf
2:  0       
3:  0       
4:  0       

back
1:  0

The problem is that you are passing the address of a local variable to your createContainer() function, but that variable goes out of scope so your container is pointing at garbage. Since this is invoking undefined behaviour, the results can be different on your machine, and both will be correct. A crash is also possible — that's one of the beauties of UB.

You need to revise it in one of two ways. Either createContainer() makes a copy of what it is passed, or you arrange to pass pointers to the variables in main() through add() to createContainer(). This code does the second — but it is probably not the better solution in the long run. However, there's (a lot) more memory management to deal with for a general solution copying what's passed.

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

typedef struct student
{
    int num;
    char *name;
} student;

typedef struct container
{
    student *data;
    struct container *next;
} container;

static container *back;
static container *front;

static container *createContainer(student *data)
{
    container *tmp = malloc(sizeof(container));
    tmp->data = data;
    tmp->next = NULL;
    return tmp;
}

static void add(student *to_add)
{
    container *tmp = createContainer(to_add);
    printf("added: (%d, %s)\n", tmp->data->num, tmp->data->name);
    back->next = tmp;
    back = tmp;
}

static void printList(container *item)
{
    for (int i = 0; item != NULL; item = item->next)
        printf("%d:\t%d\t\t%s\n", ++i, item->data->num, item->data->name);
}

int main(void)
{
    student stu1 = { 6, "grefagf" };
    student stu2 = { 3, "dsghjyreawre" };
    student stu3 = { 4, "dsghhjrant" };
    student stu4 = { 213, "fdsafgrw" };

    front = createContainer(&stu1);
    back = front;
    add(&stu2);
    add(&stu3);
    add(&stu4);

    printf("front\n");
    printList(front);
    printf("\nback\n");
    printList(back);

    return EXIT_SUCCESS;
}

There are 5 characters different here from the last version. There's a * in the function definition of add(); there's no & in the call to createContainer(); there is an & in each of the calls to add() in main(). The result is:

added: (3, dsghjyreawre)
added: (4, dsghhjrant)
added: (213, fdsafgrw)
front
1:  6       grefagf
2:  3       dsghjyreawre
3:  4       dsghhjrant
4:  213     fdsafgrw

back
1:  213     fdsafgrw

This code leaks memory because it doesn't attempt to clean up the list. That's OK for the time being. Just be aware that you'll need to clean up eventually.

edited Feb 11, 2018 at 20:30

answered Feb 11, 2018 at 20:26

Jonathan Leffler

759k145 gold badges961 silver badges1.3k bronze badges

Sign up to request clarification or add additional context in comments.

11 Comments

Alexander Over a year ago

How would I 'clean up the list'?

Jonathan Leffler Over a year ago

You'd step through from front to back, freeing the containers, and then set front and back to point to NULL once more.

static void freeList(container *item) { while (item != NULL) { container *next = item->next; free(item); item = next; } front = back = NULL; }

Alexander Over a year ago

Oh, as in like freeing the memory? Do I only need to do that once I'm finished using the list? When would a memory leak occur? (also, what is a memory leak?) Sorry for so many questions.

Jonathan Leffler Over a year ago

Yes, you free the list when you're finished with it. The memory leak occurs if your code moves on without freeing the now unused list. In this program where the list is in use until the program exits, it is not critical to free it — that's why it's "OK for the time being". However, if you're working with functions called from main(), the unreleased list becomes leaked memory when you can no longer access it. Again, with a single list maintained by global variables, it isn't an immediate problem. If you generalize the code (make it into a subroutine library), it becomes an issue.

Jonathan Leffler Over a year ago

A memory leak occurs when allocated memory can no longer be accessed but has not been freed. For example, if you have a function that allocates a string, formats some data into that string, prints it to the screen and a log file, and then returns — well, that memory is almost certainly no longer accessible and is leaked. It will never be used again, but it is still allocated to the program. For long running programs, memory leaks are a problem. For short running student exercises, they're not a major problem.

|

Alexander · Accepted Answer · 2018-02-11 20:28:12Z

1

OK i tried it is working. Only get by address student structs to your add function as below

add(&stu2);
add(&stu3);
add(&stu4);

and change function of add as below

int add(student *to_add) {

    container* tmp;
    tmp = createContainer(to_add);
    printf("added: (%d, %s)\n", tmp->data->num, tmp->data->name);

    back->next = tmp;
    back = tmp;

    return 1;
}

edited Feb 11, 2018 at 20:28

Alexander

1631 silver badge11 bronze badges

answered Feb 11, 2018 at 20:20

RedArrow

6389 silver badges20 bronze badges

Comments

Umer Amjad · Accepted Answer · 2018-02-11 20:26:43Z

1

The add function should also consume the linked list (or "container") that it is adding the "student" to.

Thus, you should not be creating a "temp" container in it, but using the passed in container and adding to the back of that passed in container.

EDIT: Just realized your back and front are global. This probably isn't a good idea, because that way you can only have a single linked list for your entire program.

Rereading now...

EDIT 2: Ahh this was a good one. You are passing in student (which makes a copy of the student) and then taking its address in the add function, which is an address of a variable on the stack. The stack is popped off so address to variables on the stack don't make sense. Instead, add should take in a pointer to a student, and you should pass in the address of stu2, stu3, etc.

EDIT 3: Ahh too late :(

edited Feb 11, 2018 at 20:26

answered Feb 11, 2018 at 19:32

Umer Amjad

687 bronze badges

4 Comments

Alexander Over a year ago

What do you mean by 'consume'? The passed in variable is the object that I want to add, and it creates a container to store it in, then adds that container to the back of the list.

Umer Amjad Over a year ago

Sorry, edited. I didn't realize your "front" and "back" pointers were global variables. (unrelated) "consume" is another way of saying a function takes as an "input", i use it to avoid confusion with I/O i.e. inputs from standard input.

Alexander Over a year ago

Ah okay. And I did have back and front passed to the function as well (not global) but it wasn't working, so I tried making them global because that's the way my professor always does it. I only need one linked list for this program anyway, so it shouldn't be an issue.

Umer Amjad Over a year ago

Edited with reply

Collectives™ on Stack Overflow

Why can I not add to a linked list correctly when using a function?

3 Answers 3

11 Comments

Comments

4 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

11 Comments

Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related