0

I am trying to create a binary search tree from a list of strings and drawing comparisons between the values by comparing the first two characters of each string. The issue is that as I iterate through the file, the root node keeps getting the value of each subsequent line. I did some digging in GDB and found that my initial "insert" function call seems to be passing the "line" or "value" parameter as a reference and so I am setting "root" to the "line" variable.

I'm struggling to figure out how to correct my program's structure to pass "line" by value instead of by reference and how to get this binary tree to be created properly. I know what's wrong but I don't know how to fix it

Output:

Input: adam 
Create node with value adam 
Root after node creation adam 
Root after insert adam

Input: bobby 
Root before insert bobby <-- This should still be "adam"
Root after compare bobby 
Compare: bo and bo 
Same 
Root after insert bobby

Input: christopher 
Root before insert christopher
Root after compare christopher 
Compare: ch and ch 
Same 
Root after insert christopher

#include <stdio.h> 
#include <stdlib.h> 
#include <string.h>
#include "node.h"

struct node_t *root = NULL;

// Insert node into the binary search tree 
void insert(struct node_t** node, char* value)
{

    char valueParameter[32];
    strcpy( valueParameter, value );

    if(!(*node)) // Node doesn't exist
    {
        printf("Create node with value %s", valueParameter);
        (*node) = (struct node_t*)malloc(sizeof(struct node_t)); // Create a
        (*node)->data = valueParameter;
        (*node)->left = NULL;
        (*node)->right = NULL;
        
        printf("Root after node creation %s", (root)->data);
    }
    else
    {
        printf("Root after compare %s", (*node)->data);
    
        // Grab the first two characters
        char newVal[32];
        strncpy(newVal, value, 2);
        newVal[2] = '\0'; 
        
        // Grab the first two characters
        char rootVal[32];
        strncpy(rootVal, (*node)->data, 2);
        rootVal[2] = '\0'; 
        
        
        printf("Compare: %s and %s\n", newVal, rootVal);
    
        if(strcmp(newVal, rootVal) < 0)
        {
            printf("Less than\n");
            insert(&(*node)->left, value);
        }
        else if(strcmp(newVal, rootVal) > 0)
        {
            printf("Greater than\n");
            insert(&(*node)->right, value);
        }
        else
        {
            printf("Same\n");
        }
    }



};

struct node_t* buildTree( char * path )
{
    
    // Opens the file
    FILE *fptr = fopen("test1.sp2021", "r");
    
    if (fptr == NULL) 
    {
        printf("Invalid input file.\n");
        return 0;
    }

    char line[32];
    while (fgets(line, sizeof(line), fptr))
    {

        printf("Input: %s", line);
        
        if (root != NULL)
        {
            printf("Root before insert %s", (root)->data);
        }
        
        // Inserts the next item into the binary search tree, starts looking for the position at the root
        insert( &root, line );
        
        printf("Root after insert %s", (root)->data);
        printf("\n");
        
    }
    
    printf("Final: %s\n", root->data);


    return root;
}
Exho
  • 113
  • 2
  • 9
  • 2
    Review `(*node)->data = valueParameter;` which is copying a pointer that will be invalid when the function ends. Exho. What do you think this line of code does? – chux - Reinstate Monica Feb 09 '21 at 18:20
  • Well taking "value" directly from the parameters and assigning it to (*node)->data, which was my initial approach, was passing the address and not simply the value. So I figured if I created another variable, copied the value to it, and set the node data to that. But that doesn't appear to be the best solution – Exho Feb 09 '21 at 21:22

1 Answers1

1

A new char array with char valueParameter[32] is created and then you are assigning (*node)->data = valueParameter;. Therefore, valueParameter is a local variable and root->data points to the stack used by the insert function, which gets destroyed when the insert function returns. Also see this question.

You have to allocate the memory of valueParameter with malloc. You should call malloc in the if statement as you do not need valueParameter when the code inside the else block is executed and you would get memory leaks.

Antonio
  • 181
  • 4