0

I am trying to make a binary search tree, but when I try to insert any value, or more precisely when a NULL pointer gets passed to the function, it just freezes for a little while and then it crashes. Here's the code:

void create(int co, struct node **leaf){
 if(*leaf==0){
   (*leaf)=malloc(sizeof(**leaf));
   (*leaf)->val=co;
   (*leaf)->left=0;
   (*leaf)->right=0;
   }
 else if(co<(*leaf)->val){
   create(co, &(*leaf)->left);
   }
 else if(co>=(*leaf)->val){
   create(co, &(*leaf)->right);
   }
}

I can't figure out why it does that. Can you explain?

EDIT: The first call of the function looks like this:

struct node *root;
 root=0;
 for(i=0;i<c;i++){
   create(f[i], &root);
   }

Where c is the number of elements in the array. And this is the definition of the struct:

struct node{
        int val;
        struct node *left;
        struct node *right;
        };

So the problem is not in the code I posted here, the entire code can be found here If I should just rewrite the whole question and just post the whole code here please saz so in the commnets, will try to rectify ASAP.

FOUND MY ANSWER After I got it to actually get past create safely, I was able to find the last one mistake that screwed up my program. It was *i++;. Apparently ++ doesn't work well with values being pointed to. After I rewrote it to *i=*i+1; it finally works, so I' like to thank all the people who helped me and ask one final question: What is the difference between *i++; and *i=i+1;?

Tomeamis
  • 477
  • 6
  • 14
  • 3
    When you say "a NULL pointer gets passed to the function", do you mean `leaf == NULL` or `*leaf == NULL`? – interjay Dec 17 '12 at 16:22
  • 1
    Shouldn't your first `if` condition be `if(*leaf == NULL)`? – noMAD Dec 17 '12 at 16:24
  • 1
    @netcoder Isn't that what has to be done? You need the space associated with `struct node`, right? – noMAD Dec 17 '12 at 16:29
  • @interjay I initialize `struct node *root; root=0;` and then pass &root as an argument. – Tomeamis Dec 17 '12 at 16:46
  • @noMAD AFAIK NULL is just a constant included in stdio.h and is defined as 0 therefore it doesn't matter whether I use NULL or 0. I might be wrong though. As for the other 2 comments I assume they are reactions to a deleted comment. – Tomeamis Dec 17 '12 at 16:47
  • 1
    @Tomeamis: `NULL` is a macro usually defined as `((void *)0)` in C. There is a slight difference, because `NULL` has pointer-type while `0` has integer-type. – netcoder Dec 17 '12 at 17:12
  • Reasons to use `NULL` instead of `0`: http://stackoverflow.com/a/177716/266392 – tomlogic Dec 17 '12 at 17:34

2 Answers2

1

There is nothing wrong with the code. Here you can see it running fine and achieving the desired results. Working on gcc 4.3.4 (C90/C99) and on gcc 4.7.2.

SomeWittyUsername
  • 18,025
  • 3
  • 42
  • 85
  • Alright, I tried to run mz whole program through the site you linked to, and I got a segfault, so the problem is probably elsewhere. [Here](http://ideone.com/qcJZyL) is the link to the whole code and the result. – Tomeamis Dec 17 '12 at 17:37
  • @Tomeamis You haven't allocated memory for your `pole` array and you try to assign values into it – SomeWittyUsername Dec 17 '12 at 17:41
  • Thanks, now it runs on the site, but in my computer it still crashes with exactly the same input. Is it possible that it's caused by the compiler? – Tomeamis Dec 17 '12 at 17:51
  • 2
    @Tomeamis: No. Only novices think that their problems are compiler bugs. Professionals know that 99.99% of their problems are not compiler bugs but programmer bugs: PEBKAC — Problem Exists Between Keyboard And Chair. No offense intended; but very very few problems are caused by compiler bugs. You should assume that it is not a compiler bug until you have the code working on half a dozen platforms with more compilers and there's just one compiler on one platform left that is crashing. Even then, it is more likely you're relying on undefined behaviour that doesn't work the same everywhere. – Jonathan Leffler Dec 17 '12 at 18:02
  • @JonathanLeffler I know that computers rarely get things wrong so it's more likely my fault but if someone else can run it and it goes without a hitch but I get errors, I can't think of another cause. – Tomeamis Dec 17 '12 at 18:18
1

I took your struct definition and insert function exactly as they were and took your other code and dumped into a main() function as such:

int main()
{
    struct node *root;
    int i, c = 10;
    root=0;
    for(i=0;i<c;i++){
        create(i, &root);
    }
    return 0;
}

Seems it works just fine. I also tried a number of different ordered elements:

int f[] = {6, 1, 9, 2, 0, 18, 2, -8, 10000, 5};

And again, no crashes and I get the correct ordering...

Did you verify that c, which you use in the condition: i<c has f[] number of elements? You could remove the c by just using: sizeof(f)/sizeof(int).

What input makes this function fail? What is the exact error message it fails with?

When you "walk" your tree, did you check for NULL before printing values?


After you posted the whole code, I can see it crashes here:

int *pole, i, count=3;
pole[0]=25;   <----

You didn't give pole any memory, so you're deferencing an uninitialized pointer.

pole = malloc(3 * sizeof(int));

fixes that, but there's more.

Next you'll die here:

void getorder(struct node *leaf, int *f, int *i){
    if(leaf->left!=NULL){
        getorder(leaf->left, f, i);
     }
     f[*i]=leaf->val;  <-- this will kill you

Because again, you don't give j any memory:

int *j;
...
*j=0;
getorder(root, f, j);
Mike
  • 47,263
  • 29
  • 113
  • 177
  • Yes, I checked. I already understand that my problem lies elsewhere. Thanks for the effort and the tip with the sizeofs. Wouldn't sizeof(f) just return the size of the pointer and not tha array though? – Tomeamis Dec 17 '12 at 17:54
  • @Tomeamis - `sizeof(f)` will return the size of the array in bytes. If it were a `char` array, that would be the number of elements (1 `char` = 1 byte). If you take `sizeof(f)/sizeof(int)` you're getting the size of your array in bytes divided by the size of an int in bytes which leaves you with number of ints in your array. – Mike Dec 17 '12 at 17:57
  • @Tomeamis - I started looking at your full code, found two errors, but there's probably more. See the edit above. It seems like you're having issues with pointers. If you use a pointer you have to assign memory (`malloc()`) before you can use it, and before you leave you should release the memory assigned (`free()`) – Mike Dec 17 '12 at 18:09