CODEWITHSUNDEEP
cpointersparameters
torrestorres
torrestorres

Reputation: 31

Pointer value being lost after function call - C

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

typedef struct Node
{
    int *data;
}Node;


void AllocateNode(Node **newnode)
{
    *newnode = malloc(sizeof(int));
}


void insert(Node **p2head, int *p2data)
{
    if(*p2head == NULL)
    {
        AllocateNode(p2head);
        (**p2head).data = p2data;
    }
}

void ReadAll(Node **headptr)
{
    int x = 10;
    insert(headptr, &x);
}

void traverse(Node *headptr)
{
    printf("%d\n",*(headptr->data));
}

int main(void)
{
    Node *ListHead;
    ListHead = NULL;
    ReadAll(&ListHead);
    printf("%d\n",*(ListHead->data));

    traverse(ListHead);
}

I am very confused because

printf("%d\n",*(ListHead->data));

outputs: 10 - the desired value, however

printf("%d\n",*(headptr->data));

outputs: 0 - the value is randomly lost after being passed to the traverse function, even though it seems to be assigned correctly after all the other calls.

Upvotes: 3

Views: 720

Answers (2)

bruno
bruno

Reputation: 32586

There are several problems in your code :

In

void AllocateNode(Node **newnode)
{
    *newnode = malloc(sizeof(int));
}

*newnode is a Node* so it must be

void AllocateNode(Node **newnode)
{
    *newnode = malloc(sizeof(Node));
}

In

void ReadAll(Node **headptr)
{
    int x = 10;
    insert(headptr, &x);
}

you give the address of x being a local variable to save it in the Node.

All access through that pointer is invalid and has an unspecified behavior when you exit ReadAll

One possibility is to allocate the int

void ReadAll(Node **headptr)
{
    int * x = malloc(sizeof(int));

    *x = 10;
    insert(headptr, x);
}

After these corrections, compilation and execution :

vxl15036 /tmp % gcc -pedantic -Wextra -g l.c
vxl15036 /tmp % ./a.out
10
10

Execution under valgrind

vxl15036 /tmp % valgrind ./a.out
==28709== Memcheck, a memory error detector
==28709== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==28709== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==28709== Command: ./a.out
==28709== 
10
10
==28709== 
==28709== HEAP SUMMARY:
==28709==     in use at exit: 12 bytes in 2 blocks
==28709==   total heap usage: 2 allocs, 0 frees, 12 bytes allocated
==28709== 
==28709== LEAK SUMMARY:
==28709==    definitely lost: 8 bytes in 1 blocks
==28709==    indirectly lost: 4 bytes in 1 blocks
==28709==      possibly lost: 0 bytes in 0 blocks
==28709==    still reachable: 0 bytes in 0 blocks
==28709==         suppressed: 0 bytes in 0 blocks
==28709== Rerun with --leak-check=full to see details of leaked memory
==28709== 
==28709== For counts of detected and suppressed errors, rerun with: -v
==28709== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 6)

Upvotes: 0

MikeCAT
MikeCAT

Reputation: 75062

An pointer to non-static local variable is passed from ReadAll() to insert() and it is saved to the newly created node.

This variable becomes unavailable after returning from ReadAll() and dereferencing the pointer after that invokes undefined behavior. This is the cause of randomness.

To avoid this, the pointer to put on the node should be that points to object that is available even after returning from ReadAll().

This can be archived by dynamically allocating

void ReadAll(Node **headptr)
{
    int *x = malloc(sizeof(int));
    *x = 10;
    insert(headptr, x);
}

or making the variable static.

void ReadAll(Node **headptr)
{
    static int x = 10;
    insert(headptr, &x);
}

Also, the implementation of Allocate() is wrong as Peter pointed out. The allocation size should be sizeof(Node), not sizeof(int).

Upvotes: 1

Related Questions