Search code examples
ctreebinary-treebinary-search-treebinary-search

How do I insert a new node into a binary tree in C?


I've been trying to get this to work for a while now, but there is clearly something I don't understand.

I have to insert a new node into a binary tree with "phone" as it's value.

void bst_insert_node(bstree* bst, unsigned long phone, char *name) {
    bst_node* new_node = malloc(sizeof(bst_node));
    new_node->phone = phone;
    bst_node* y = NULL;
    bst_node* x = bst->root;

    while(x != NULL)
    {
        y = x;
        if(phone <= x->phone)
             x = x->left;
        else x = x->right;
    }
    if (y == NULL)
    {
        bst->root = new_node;
    }
    else
    {
        if (phone <= y->phone)
             y->left->phone = phone;
        else y->right->phone = phone;
    }
}

I would appreciate any type of comments and explanations.

Thank you very much in advance.

Edit: I have corrected y->left->phone = phone to y->left = new_node as one of your comments suggested. Still, it does not work. Valgrind gives me this:

valgrind ./a.out telefonbuch_einfach.txt ==5941== Memcheck, a memory error detector
==5941== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==5941== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==5941== Command: ./a.out telefonbuch_einfach.txt
==5941== 
==5941== error calling PR_SET_PTRACER, vgdb might block
==5941== Conditional jump or move depends on uninitialised value(s)
==5941==    at 0x4013AB: bst_insert_node (introprog_telefonbuch.c:19)
==5941==    by 0x400A5E: read_file (introprog_main_telefonbuch.c:54)
==5941==    by 0x400CB6: main (introprog_main_telefonbuch.c:119)
==5941== 
==5941== Use of uninitialised value of size 8
==5941==    at 0x401435: bst_insert_node (introprog_telefonbuch.c:31)
==5941==    by 0x400A5E: read_file (introprog_main_telefonbuch.c:54)
==5941==    by 0x400CB6: main (introprog_main_telefonbuch.c:119)
==5941== 
==5941== Invalid write of size 8
==5941==    at 0x401435: bst_insert_node (introprog_telefonbuch.c:31)
==5941==    by 0x400A5E: read_file (introprog_main_telefonbuch.c:54)
==5941==    by 0x400CB6: main (introprog_main_telefonbuch.c:119)
==5941==  Address 0x18 is not stack'd, malloc'd or (recently) free'd
==5941== 
==5941== 
==5941== Process terminating with default action of signal 11 (SIGSEGV)
==5941==  Access not within mapped region at address 0x18
==5941==    at 0x401435: bst_insert_node (introprog_telefonbuch.c:31)
==5941==    by 0x400A5E: read_file (introprog_main_telefonbuch.c:54)
==5941==    by 0x400CB6: main (introprog_main_telefonbuch.c:119)
==5941==  If you believe this happened as a result of a stack
==5941==  overflow in your program's main thread (unlikely but
==5941==  possible), you can try to increase the size of the
==5941==  main thread stack using the --main-stacksize= flag.
==5941==  The main thread stack size used in this run was 8388608.
==5941== 
==5941== HEAP SUMMARY:
==5941==     in use at exit: 832 bytes in 6 blocks
==5941==   total heap usage: 7 allocs, 1 frees, 4,928 bytes allocated
==5941== 
==5941== LEAK SUMMARY:
==5941==    definitely lost: 0 bytes in 0 blocks
==5941==    indirectly lost: 0 bytes in 0 blocks
==5941==      possibly lost: 0 bytes in 0 blocks
==5941==    still reachable: 832 bytes in 6 blocks
==5941==         suppressed: 0 bytes in 0 blocks
==5941== Rerun with --leak-check=full to see details of leaked memory
==5941== 
==5941== For counts of detected and suppressed errors, rerun with: -v
==5941== Use --track-origins=yes to see where uninitialised values come from
==5941== ERROR SUMMARY: 5 errors from 3 contexts (suppressed: 0 from 0)
Segmentation fault (core dumped)

Solution

  • For starters there is a typo

    else
    {
        if (phone <= y->phone)
             y->left->phone = phone;
        else y->right->phone = phone;
    }
    

    There should be

    else
    {
        if (phone <= y->phone)
             y->left = new_node;
        else y->right = new_node;
    }
    

    The function does not use the parameter name.

    Also data members left and right are not initialized for the newly created node.

    The function can be defined the following way as it is shown below. I assume that the corresponding structures are defined like

    typedef struct bst_node
    {
        unsigned long phone;
        char *name;
        struct bst_node *left;
        struct bst_node *right;
    } bst_node;
    
    typedef struct
    {
        bst_node *root;
    } bstree;
    

    Here is the function.

    int bst_insert_node( bstree* bst, unsigned long phone, const char *name ) 
    {
        bst_node *new_node = malloc( sizeof( bst_node ) );
        int success = new_node != NULL;
    
        if ( success )
        {
            new_node->name = malloc( strlen( name ) + 1 );
    
            success = new_node->name != NULL;
    
            if ( !success ) free( new_node );
        }
    
        if ( success )
        {
            strcpy( new_node->name, name );
            new_node->phone = phone;
            new_node->left = NULL;
            new_node->right = NULL;
    
            bst_node **current = &bst->root;
    
            while ( *current != NULL )
            {
                if ( new_node->phone < ( *current )->phone )
                {
                    current = &( *current )->left;
                }
                else
                {
                    current = &( *current )->right;
                }
            }
    
            *current = new_node;
        }
    
        return success;
    }       
    

    If the data member name is a character array then the function will look simpler because you will not need to allocate memory to store name.