Segmentation fault when deleting node from BST, using double pointers

Hi Guys, can you please point out what could be the issue with code below. I guess I m doing something wrong with pointers maintenance however from my perspective this code should be ok.

What I am doing is:
1. Create tree and populate it with nodes. -- OK
2. Print tree. --- OK
3. Locate node to be removed. -- OK
3. Delete particular node (logic seems to be OK) however I get segmentation fault all the time once I am trying to free() node that has to be removed.

Please help.

Code:

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

typedef struct node *tree;

struct node {
    int key;
    tree left;
    tree right;
};

void InsertKey(tree *tr, int key) {
    if (*tr == NULL) {
        if ( (*tr=(tree)malloc(sizeof(tree))) == NULL) {
            printf("Can not allocate root\n");
            exit(1);
        }
        (*tr)->key=key;
        (*tr)->left=NULL;
        (*tr)->right=NULL;    
    }
    else {
        if (key<(*tr)->key)
            InsertKey(&(*tr)->left, key);
        else
            InsertKey(&(*tr)->right, key); 
    }
}

void PrintKey(tree *tr) {
    if (*tr == NULL)
        return;
    else {
    printf("Value of item is %d\n", (*tr)->key);
    PrintKey(&(*tr)->left);
    /*printf("Value of item is %d\n", (*tr)->key);*/
    PrintKey(&(*tr)->right);
    }
}

tree SearchTree(tree tr, int key) {
	if (tr==NULL) /* tree is empty*/
		return NULL;
	else if (tr->key>key) /* search left subtree*/
		return SearchTree(tr->left, key);
	else if (tr->key<key) /* search right subtree*/
		return SearchTree(tr->right, key);
	else
		return tr;
}

void DeleteNodeR(tree *tr) {
	if ((*tr)->left==NULL && (*tr)->right==NULL) {
		tree tmp=*tr;
		*tr=NULL;
		free(tmp);
		tmp=NULL;
	}
	else if ((*tr)->left==NULL) {
		tree root=*tr;
		*tr=(*tr)->right;
		free(root);
		
	}
	else if ((*tr)->right==NULL) {
		tree root=*tr;
		*tr=(*tr)->left;
		free(root);
		
	}
	else { /* node has both children, get right most node from left subtree */
		tree iterateLeftTree=(*tr)->left;
		while (iterateLeftTree->right!=NULL)
			iterateLeftTree=iterateLeftTree->right;
		(*tr)->key=iterateLeftTree->key;
		free(iterateLeftTree);
		iterateLeftTree=NULL;
	}
}

int main(void) {
    tree tr=NULL;
    tree locatedNode=NULL;

    InsertKey(&tr, 15);
    InsertKey(&tr, 20);
    InsertKey(&tr, 10);

    PrintKey(&tr);
    locatedNode=SearchTree(tr,10);
    printf("Remove node %d\nTree after removal\n", locatedNode->key);
    DeleteNodeR(&locatedNode);
    PrintKey(&tr);
    return 0;
}

Thank you.

Code:

typedef struct node *tree;

Please don't do that. That is the worst possible way to abuse a pointer typedef.
It's better to remove it completely and use node** instead. No confusion there.
Consider it a suggestion.

Feel free to ignore what Elysia just wrote. You're not doing anything wrong. That one poster just happens to dislike that style.
I shudder to think what Elysia thinks of the Windows API that is full of pointer typedefs like LPCSTR.

I'm not seeing any obvious problem, but I haven't run your program yet.

Feel free to ignore iMalc.
Pointer typedefs complicate the program and makes it prone to errors. Plus it's MORE to type than NOT using them (usually).
Using pointer typedefs will bite you later. ESPECIALLY a pointer typedef that is inapproperitly named (as yours is).
A tree is not a pointer; it's an object.

So let me ask another question instead. Why is it so hard to do
typedef struct node node;
And do
node**
instead?

Typedefs are good at hiding complex syntax into something more readable, but this obfuscates the reading by hiding the fact that it's a pointer, so it makes it harder to read.

In this case the bug is actually caused by this line:

Code:

        if ( (*tr=(tree)malloc(sizeof(tree))) == NULL) {

sizeof(tree) is the size of a pointer. You want sizeof(*tree) or sizeof(struct node).

Regardless of the fact that it possibly did hide the error in this case, it's up to you whether you remove the typedef or not. I can certainly agree that it wasn't named in the best manner, but that isn't grounds for getting rid of it, more for giving it a better name.

Note that I'm not demanding that you should get rid of it. It's a suggestion. Your typedef hid a little problem right there because it was inappropriately named.

The OP probably could easily have made more errors (but for different reasons) if he had to deal with ** all over the place.

It was badly written. Since you "reacted" I edited it to outline it in a nicer way, to add that it was a suggestion. The way it should be.

Alright, I'll concede to that.
I guess I started with an opening that got your back up, and it went downhill from there.