CODEWITHSUNDEEP
cstruct
Stephen
Stephen

Reputation: 6087

Failing to build a 'queue-like' structure - problems with struct declarations?

Hey guys. This is a very simple question, I'm sure, but I'm getting myself tangled up in C references/pointers as per usual. I am trying to build a... sort-of-queue, using a sort-of-linked list. Basically, I have a struct which has contents and a pointer to the next element. I also have a pointer to the first and last elements. I then have a loop that will be building the 'sort-of-queue'. My problem is that either my logic is failing and I'm not initialising the queue right, or my knowledge of C structs is failing (which is very probable) and I'm ending up just creating one struct and constantly referring to it.

My test code is as follows:

#include <stdio.h>

struct test {
    int contents;
    struct test *next;
};

main() {

    struct test *first = NULL;
    struct test *last = NULL;
    int i;

    for (i = 0; i < 2; i++) {
        struct test tmp;
        if (first == NULL) {
            first = &tmp;
            last = &tmp;
        } else {
            last->next = &tmp;
            last = &tmp;
        }
        tmp.x = i;
        tmp.next = NULL;
    }

    while (first != NULL) {
        printf("%d\n", first->x);
        first = first->next;
    }

    return 0;

}

Running this, I get the output that first seems to point to a test struct that has the value of '1' as it's 'x' variable - so not the initial one like I intended. So, am I failing at logic here, or am I failing at understanding how to declare new separate structs in a loop? Or maybe both? I'm very tired... >_<.

Thanks.

Upvotes: 1

Views: 96

Answers (1)

JaredPar
JaredPar

Reputation: 754575

The problem you have is that you are taking the address of a temporary variable, tmp, and assigning it to a pointer which lives much longer than teh temporary, first and last. After every iteration of the loop the temporary is gone and continuing to access it via first and last results in undefined behavior.

You need to create a value on the heap in order to build up the list like so (error checking omitted for brevity)

struct test* tmp = malloc(sizeof(struct test));

Later though you'll need to go through and free all of the allocated nodes.

Upvotes: 2

Related Questions