wrong linked list value

[hannibar]

hi all,

I have a problem with linked lists.
I have written this code :

//in main()
    Vlak *vl1;
    vl1 = (Vlak *)malloc(sizeof(Vlak));
    vl1->v1[0] = 1.0f;
    initVlakList();
    addVlakList(vl1);

...

void initVlakList(){
    VlakList *v;
    Vlak *f, *l;

    v = (VlakList *)malloc(sizeof(VlakList));
    f = (Vlak *)malloc(sizeof(Vlak));
    l = (Vlak *)malloc(sizeof(Vlak));

    v->first = f;
    v->last = l;

    G->vlak = v;
}

...

void addVlakList(Vlak *v){
    Vlak *first, *last;
    first = G->vlak->first;
    last = G->vlak->last;

    if ((last == 0) && (first == 0)){
        last = v;
        first = v;
    }
    else {
        last->next = v;
        v->prev = last;
        last = v;
    }
}

Vlak and VlakList are defined like this :

typedef struct Vlak{
    struct Vlak *next;
    struct Vlak *prev;

    float v1[3], v2[3], v3[3], v4[3];
}Vlak;

typedef struct VlakList {
    struct Vlak *first;
    struct Vlak *last;
}VlakList;

and G contains a pointer to a VlakList : G->vlak .

When I call this :
value = G->vlak->first->v1[0];
, it gives a wrong result (waaaaaaay to high).

Does anyone know what the problem could be ?

Thanks in advance.

[DavT]

Um... where to start...
initVlakList() creates a VlakList and two Vlaks (which are uninitialised) and makes these the first and last items in the VlakList. Basically that is your mistake. When you printed out G->vlak->first->v1[0] you weren't printing what you added but one of these uninitalied Vlaks. Do this instead:

/* make an empty list */
void initVlakList()
{
  VlakList *v;

  v = (VlakList *)malloc(sizeof(VlakList));
  v->first = NULL;
  v->last = NULL;
  G->vlak = v;
}

Also, in addVlakList() last and first were temporary copies of what you wanted to make changes to and so your list wasn't really being updated. (oh and use NULL rather than 0 for pointers). Updated it looks like this:

void addVlakList(Vlak *v)
{
  if ((G->vlak->last == NULL) && (G->vlak->first == NULL)){
    G->vlak->last = v;
    G->vlak->first = v;
  }
  else {
    G->vlak->last->next = v;
    v->prev = G->vlak->last;
    G->vlak->last = v;
  }
}

::edit:: oops, screwed up the code tags - making the update before anyone notices...

[Salem]

Impossible to tell from the random snippets of code you've posted.

One obvious thing is to stop casting the return result of malloc (see the FAQ)

[hannibar]

thanks DavT, that did it.