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 开发者_Go百科*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

---

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!

---

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.

---

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.

---

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;
  }