How does this Binary Search Tree look? I wrote a BST in C a while back and may use it at some point. 

 search_tree tree_make_empty( search_tree tree )
 {
   if ( tree != NULL )
   {
       tree_make_empty( tree-&gt;left );
       tree_make_empty( tree-&gt;right );
       free( tree );
   }
   return NULL;
 }

 tree_position tree_find( CHAR_DATA *target, search_tree tree )
 {
     if ( tree == NULL )
       return NULL;

     if ( target &lt; tree-&gt;hatedata-&gt;target_char )
       return tree_find( target, tree-&gt;left );
     else if ( target &gt; tree-&gt;hatedata-&gt;target_char )
       return tree_find( target, tree-&gt;right );
     else
       return tree;
 }

 search_tree tree_insert( HATE_DATA *hatedata, search_tree tree )
 {
     if ( tree == NULL )
     {
       tree = (HATE_NODE * ) malloc( sizeof( HATE_NODE ) );

       if ( tree == NULL )
          bug( "tree_insert: out of space!" );
       else
       {
          tree-&gt;hatedata = hatedata;
          tree-&gt;left = tree-&gt;right = NULL;
       }
     }
     else if ( hatedata-&gt;target_char &lt; tree-&gt;hatedata-&gt;target_char )
       tree-&gt;left = tree_insert( hatedata, tree-&gt;left );
     else if ( hatedata-&gt;target_char &gt; tree-&gt;hatedata-&gt;target_char )
          tree-&gt;right = tree_insert( hatedata, tree-&gt;right );

     return tree;
 }

 tree_position tree_find_min( search_tree tree )
 {
    if ( tree == NULL )
       return NULL;
    else if ( tree-&gt;left == NULL )
       return tree;
    else
       return tree_find_min( tree-&gt;left );
 }

 search_tree tree_delete( HATE_DATA *hatedata, search_tree tree )
 {
    tree_position pos;

    if ( tree == NULL )
       bug( "tree_delete: not found" );
    else if ( hatedata-&gt;target_char &lt; tree-&gt;hatedata-&gt;target_char )
       tree-&gt;left = tree_delete( hatedata, tree-&gt;left );
    else if ( hatedata-&gt;target_char &gt; tree-&gt;hatedata-&gt;target_char )
         tree-&gt;right = tree_delete( hatedata, tree-&gt;right );
    else if ( tree-&gt;left &amp;&amp; tree-&gt;right )
    {
       pos = tree_find_min( tree-&gt;right );
       tree-&gt;hatedata = pos-&gt;hatedata;
       tree-&gt;right = tree_delete( tree-&gt;hatedata, tree-&gt;right );
    }
    else
    {
       pos = tree;
       if ( tree-&gt;left == NULL )
         tree = tree-&gt;right;
       else if ( tree-&gt;right == NULL )
         tree = tree-&gt;left;
       free( pos );
    }

    return tree;
 }

 HATE_DATA *tree_retrieve( tree_position pos )
 {
    return pos-&gt;hatedata;
 }


...

 struct hate_data
 {
    CHAR_DATA *target_char;
    int hate_amount;
 };

 struct hate_node
 {
    HATE_DATA *hatedata;
    search_tree left;
    search_tree right;
 };


...

mob-&gt;hatedata = tree_make_empty( NULL );


Example use:

if ( IS_NPC(victim) )
{
     HATE_DATA *hatedata;
   tree_position P;

   if( ( P = tree_find( ch, victim-&gt;hatedata )) == NULL || tree_retrieve( P )-&gt;target_char != ch )
   {
     int test;
     hatedata = (HATE_DATA * ) malloc( sizeof( HATE_DATA ) );
     hatedata-&gt;target_char = ch;
     test = number_range( 1, 50 );
     hatedata-&gt;hate_amount = test;
     victim-&gt;hatedata = tree_insert( hatedata, victim-&gt;hatedata );
     ch_printf( ch, "It should now hate you for %d.\n\r", test );
   }
   else
   {
     hatedata = tree_retrieve(tree_find( ch, victim-&gt;hatedata ));
     ch_printf(ch, "You are already hated for %d!\n\r", hatedata-&gt;hate_amount );
   }

}


Do you have any suggestions? Does it look okay? Are there any ways to optimize it?
In tree_find and tree_find_min [edit: and even in tree_insert] you're not really gaining anything from using recursion. For example, I think tree_find_min would probably be clearer something like:

tree_position tree_find_min( search_tree tree )
{
   if ( tree == NULL )
      return NULL;
   while (tree-&gt;left != NULL)
       tree = tree-&gt;left;
   return tree;
}


As a side-benefit, this may also be faster with some compilers. In code like:

 HATE_DATA *hatedata;

 /* ... */

 hatedata = (HATE_DATA * ) malloc( sizeof( HATE_DATA ) );


I'd change it to look more like:

 hatedata = malloc(sizeof(*hatedata));


The cast accomplishes nothing useful in C, and can cover up the bug of forgetting to #include &lt;stdlib.h&gt; to get the proper prototype for malloc. Using sizeof(*hatedata) instead of sizeof(HATE_DATA) means that changing the type only requires changing it in one place (where you've defined the variable), instead of everywhere you've done an allocation.