CODEWITHSUNDEEP
cpointersstructlinked-list
Brandon Coffman
Brandon Coffman

Reputation: 244

Odd problem with pointer while implementing a linked list

I'm trying to implement a linked list in C, and I want to store the head node in a separate struct. However, it seems like the head node is being reassigned somehow whenever I add another node.

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

struct BC_node {
    struct BC_node *next;
    void *data;
};
struct BC_list {
    struct BC_node *head;
    struct BC_node *tail;
};

void
BC_list_push(struct BC_list *list, void *data)
{
    struct BC_node *node = calloc(1, sizeof(struct BC_node));

    if (list->head != NULL)
        printf("head: %d\n", *((int *) (list->head)->data));

    node->next = NULL;
    node->data = data;
    if (list->head == NULL) {
        printf("head is null.\n");
        list->head = node;
    }
    if (list->tail != NULL) {
        (list->tail)->next = node;
    }
    list->tail = node;

    printf("head: %d\n", *((int *) (list->head)->data));
}

int
main(void)
{
    int i;
    struct BC_list *list = calloc(1, sizeof(struct BC_list));

    list->head = NULL;
    list->tail = NULL;
    for (i = 0; i < 3; i++)
        BC_list_push(list, &i);
    return 0;
}

The output:

head is null.
head: 0
head: 1
head: 1
head: 2
head: 2

Upvotes: 0

Views: 247

Answers (4)

cendar
cendar

Reputation: 34

Here a full example with your struct and memory allocation for the data,

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

struct BC_node{
    void *data;
    struct BC_node *next;
};
struct BC_list{
    struct BC_node *head;
    struct BC_node *tail;
};

struct BC_node *BC_new_node(void *data,size_t szdata)
  {
    struct BC_node *ptr=malloc(sizeof(struct BC_node));
    if(ptr!=NULL){
        ptr->data=malloc(szdata);
        ptr->next=NULL;
        if(ptr->data!=NULL)
            memcpy(ptr->data,data,szdata);
        else free(ptr),ptr=NULL;
    }
    return ptr;
  }

int BC_list_push(struct BC_list *list,void *data,size_t szdata)
  {
    struct BC_node *ptr=BC_new_node(data,szdata);
    if(list!=NULL && ptr!=NULL){
        if(list->tail==NULL){
            list->head=ptr;
            list->tail=ptr;
        } else {
            list->tail->next=ptr;
            list->tail=ptr;
        }
        return 0;
    }
    return 1;
  }

void *BC_new_list(void)
  {
    struct BC_list *ptr=malloc(sizeof(struct BC_list));
    if(ptr!=NULL)
        ptr->head=ptr->tail=NULL;
    return ptr;
  }

void BC_free_list(struct BC_list *list)
  {
    struct BC_node *ptr;
    while(list->head){
        ptr=list->head->next;
        free(list->head->data);
        free(list->head);
        list->head=ptr;
    }
    free(list);
  }

void print_test(struct BC_list *list){
    struct BC_node *ptr=list->head;
    while(ptr){
        printf("%s\n",(char*)ptr->data);
        ptr=ptr->next;
    }
}

int main(void)
  {
    char tab[3][40]={"hello","world","test"};
    struct BC_list *list=BC_new_list();
    if(list!=NULL){
        BC_list_push(list,tab[0],strlen(tab[0])+1);
        BC_list_push(list,tab[1],strlen(tab[1])+1);
        BC_list_push(list,tab[2],strlen(tab[2])+1);
        print_test(list);
        BC_free_list(list);
    }
    return EXIT_SUCCESS;
  }

Upvotes: 1

Jarek Potiuk
Jarek Potiuk

Reputation: 20047

It's worse. &i is address of a local variable, which means that you refer from a heap-allocated list to a stack, volatile variable... This is purely wrong - if you do it in a method different than main, then the local variable you have will disappear and your pointer will point to some random place in memory, possibly pointing at other variables or addresses ... very bad.

Upvotes: 2

Node
Node

Reputation: 3509

The problem is this line:

BC_list_push(list, &i);

You are passing the address of i, which is the integer you are incrementing in the loop (so the value changes). You need to allocate separate memory for the data argument.

for (i = 0; i < 3; i++)
{
    int *d = malloc(sizeof(int));
    *d = i;
    BC_list_push(list, d);
}

But don't forget to free the memory when the list is destroyed.

Upvotes: 2

Kerrek SB
Kerrek SB

Reputation: 476910

Your data member is just a pointer to the variable i in main, so when you print *data you just see the value of the counter during that round of the loop. All your nodes have the same data value!

Upvotes: 6

Related Questions