0

I have this piece of code:

struct nodo {
    char c;
    int val;
    struct nodo *pizq, *pder;
};
typedef struct nodo NODO;

With its proper initializer function:

NODO * crea_nodo_int (int valor ) 
{
    NODO *p;
    p = (NODO *)malloc( sizeof( NODO ) );
    if (p == NULL )
        return NULL;
    p -> pizq = NULL;
    p -> pder = NULL;

    p -> val = valor;
    return p;
}

On the main function I have:

NODO * test = crea_nodo_int(5);
free(test);
inorder(test);

As you can see it's an implementation of a binary tree where pizq and pder are left and right children respectively.

However I'm posting this because I had understood that 'free()' actually deallocates memory and I can't see this here. You see, I create node test with value of 5, then free it, then when I walk inorder in the console it prints '0' and terminates. Why the hell is printing 0? Shouldn't it go straight to the case where if (p == NULL) return and terminate without printing anything? What am I doing wrong?

Here's my code for the inorder function:

void inorder( NODO *p )
{
    if (p == NULL) return;

    if( p->pizq != NULL )
        inorder(( p->pizq );

    // Process root
    printf("%d ", p->val );

    if( p->pder != NULL )
        inorder( p->pder );
}
David Merinos
  • 1,195
  • 1
  • 14
  • 36

2 Answers2

4

free(ptr) deallocates the memory ptr pointer is pointing to but ptr still points to the freed memory location and accessing such memory is undefined behavior.

Check this.

Notice that this (free)function does not change the value of ptr itself, hence it still points to the same (now invalid) location.

So, just calling free() will make the pointer a Dangling Pointer that do not point to a valid object of the appropriate type.

After calling free(ptr) follow a practice of setting the pointer to NULL:

ptr = NULL;

This will protect against the dangling pointer bugs.

So, in your case, you should -

free(test);
test = NULL;
H.S.
  • 11,654
  • 2
  • 15
  • 32
  • So is there a way to do this? How can I avoid that my inorder walk actually prints something? It's going into the `printf("%d ", p->val );` line which seems un natural to me. – David Merinos Oct 28 '17 at 06:05
  • @DavidMerinos, always remember that **all parameters to functions in C are passed by value** so no way for a function to change the pointer passed to `free(3)`, it will continue pointing to the place you have freed, if you don't take the appropiate measures. – Luis Colorado Nov 01 '17 at 10:39
2

Once you have called free on some pointer, you are not allowed to do anything on that memory zone. So you should not call inorder (or you might call it before free).

Your program has undefined behavior. Be very scared. Hence its behavior is unpredictable and not explainable (unless you spend years diving into implementation details).

I recommend having several constructor-like functions:

NODE*create_leaf_node(int val);
NODE*create_pair_node(NODE*left, NODE*right);

and write a (recursive) destructor-like function (which ends by calling free(n);):

void destroy_node(NODE*n);

Of course, I suppose you have your struct nodo as in your question, and

typedef struct nodo NODE;

Your test program might do:

NODE*ntest = create_pair_node(create_pair_node(create_leaf_node(1),
                                               NULL),
                              create_leaf_node(3));
inorder(ntest);
destroy_node(ntest), ntest = NULL;

(I'm using the comma operator in the last statement)

Then compile your program with all warnings and debug info (e.g. gcc -Wall -Wextra -g with GCC). Improve it to get no warnings. Use the debugger gdb (and perhaps other tools, like valgrind).

BTW, in C you should not cast the result of malloc ... You might consider displaying (with perror) some error message when malloc fails then exiting the program, like here.

Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547