Malloc function in C errors with pointers

Question

I've create this function that's supposed to create a randomly generated binary tree, it works fine but at the end of the function the root == NULL, i can't understand why!

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

#define MAX_B 7

typedef struct _ramo{
    int nbanane;
    struct _ramo *dx;
    struct _ramo *sx;
}ramo;

void creaAlbero(ramo *root, int n){
    printf("%d\n",n);
    root = malloc(sizeof(ramo));
    root->nbanane=rand()%MAX_B;
    printf("BANANA! %d\n",root->nbanane);
    root->dx=NULL;
    root->sx=NULL;
    if ((int)(rand()%n)==0)
        creaAlbero(root->dx, n+1);
    if ((int)(rand()%n)==0)
        creaAlbero(root->sx, n+1);
 }

int main(){
    srand((unsigned int)time(NULL));
    ramo *root=NULL;
    creaAlbero(root, 1);
    if (root==NULL) {
        printf("EMPTY!!");
    }
    return 0;
}

C is pass by value, you need a pointer to a pointer parameter in creaAlbero to modify your root object in main. — ouah
– ouah, Commented Jan 15, 2015 at 20:38
the code needs to check the returned value from a call to malloc to assure successful operation — user3629249
– user3629249, Commented Jan 15, 2015 at 21:46
the function: 'creaAlbero' could recurse forever if the result of "if ((int)(rand()%n)==0)" was never 0 — user3629249
– user3629249, Commented Jan 15, 2015 at 21:50
@user3629249 that's true before i've had a limitation but i've noticed that it's pretty rare to get "n" over 5 so i've take it off — Alberto Fiocco
– Alberto Fiocco, Commented Jan 16, 2015 at 1:17

Paul Roub · Accepted Answer · 2015-01-15 20:41:15Z

4

You set root to NULL:

ramo *root=NULL;

then pass a copy of it to creaAlbero():

creaAlbero(root, 1);

which modifies the copy

root = malloc(sizeof(ramo));

then returns. The original root is still NULL, because nothing changed it.

Consider returning root from creaAlbero():

ramo * creaAlbero(int n){
  printf("%d\n",n);

  ramo *root = malloc(sizeof(ramo));
  root->nbanane=rand()%MAX_B;
  printf("BANANA! %d\n",root->nbanane);
  root->dx=NULL;
  root->sx=NULL;

  if ((int)(rand()%n)==0)
    root->dx = creaAlbero(n+1);
  if ((int)(rand()%n)==0)
    root->sx = creaAlbero(n+1);

  return root;
}

int main(){
  srand((unsigned int)time(NULL));
  ramo *root=NULL;
  root = creaAlbero(1);
  if (root==NULL) {
    printf("EMPTY!!");
  }
  return 0;
}

Example: https://ideone.com/dXiv8A

answered Jan 15, 2015 at 20:41

Paul Roub

36.5k27 gold badges88 silver badges95 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Community · Accepted Answer · 2017-05-23 12:20:41Z

creaAlbero(ramo *root, int n) is a function that takes a copy of a pointer to a ramo. It then proceeds to do stuff with this copy of the pointer, and then returns. main then looks a the value of the origonal root variable, which was (obviously) never changed.

If you want a function to modify a value that's passed in, you must pass the object by pointer. To clarify: if you want a function to modify a pointer, the function must take as a parameter a pointer to a pointer to a thing:

void creaAlbero(ramo **rootptr, int n){     //pass rootptr by pointer     
    *rootptr = malloc(sizeof(ramo)); //modify pointer pointed at by rootptr
    ramo* root = *rootptr; //make local copy of value for ease of use
    //rest of your code here
}
int main(){
    ramo *root=NULL;
    creaAlbero(&root, 1);  //pass by pointer

Paul Roub's answer also suggests another excellent idea: return the ramo* from the function instead of taking it as a mutable parameter. It's simpler and more intuitive by far.

R Sahu · Accepted Answer · 2015-01-15 20:44:34Z

root is passed to creaAlbero by value. Any changes made to root in creaAlbero are only local modifications. They don't change the value of root in main. A better alternative would be to change the signature of creaAlbero to:

ramo* creaAlbero(int n){
   printf("%d\n",n);
   ramo* root = malloc(sizeof(ramo));
   root->nbanane=rand()%MAX_B;
   printf("BANANA! %d\n",root->nbanane);
   root->dx=NULL;
   root->sx=NULL;
   if ((int)(rand()%n)==0)
      root->dx = creaAlbero(n+1);
   if ((int)(rand()%n)==0)
      root->sx = creaAlbero(n+1);

   return root;
}

and change the usage to:

int main(){
   srand((unsigned int)time(NULL));
   ramo *root = creaAlbero(1);
   if (root==NULL) {
      printf("EMPTY!!");
   }
   return 0;
}

Collectives™ on Stack Overflow

Malloc function in C errors with pointers

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related