Segmentation fault on binary tree-CodePudding

I'm working on a program that searches through a program and outputs the number of times it comes across a number greater than or equal to the value specified in the parameter of the function. I have attached my code below, I see it traverses two of the values before a segmentation fault occurs. The segmentation fault error points to return count count_values(value, here) count_values(value, current);

typedef struct BinaryTree {
    int val;
    struct BinaryTree *left;
    struct BinaryTree *right;
} BinaryTree;

BinaryTree *build_tree(int value, BinaryTree *leftnode, BinaryTree *rightnode) {
    BinaryTree *out = calloc(1, sizeof(BinaryTree));
    out->val = value;
    out->leftnode = leftnode;
    out->rightnode = rightnode;

    return out;
}

int count_values(int value, BinaryTree *tree) {
  
    BinaryTree *current = tree;
    BinaryTree *here = current;
    int count = 0;

    if (current != NULL) {
        printf("Value in tree is not NULL\n");

        if (current->val < value) {
            printf("%d < %d\n", current->val, value);
            printf("Count value: %d\n", count);
            here = current->leftnode;
            count_values(value, here);
            current = current->rightnode;
            count_values(value, current);

        } else if (current->val == value) {
            printf("%d = %d\n", current->val, value);
            count  ;
            printf("Count value: %d\n", count);
            here = current->leftnode;
            count_values(value, here);
            current = current->rightnode;
            count_values(value, current);
        } else {
            printf("%d > %d\n", current->val, value);
            count  ;
            printf("Count value: %d\n", count);
            here = current->leftnode;
            count_values(value, here);
            current = current->rightnode;
            count_values(value, current);
        }
    }
        
    return count   count_values(value, here)   count_values(value, current);
}

int main(void) {
    BinaryTree *tree =
        build_tree(14, build_tree(3, NULL, NULL),
                   build_tree(15, NULL, build_tree(42, NULL, NULL)));

    //path is 14->15->42
    int count = count_values(42, tree);
    printf("should have a count of 1, got %d\n", count);

    count = count_values(14, tree);
    printf("should have a count of 3, got %d\n", count);

    return 0;
}

CodePudding user response：

You're making this way harder than it needs to be.

First test for the base case by testing whether tree is NULL.

If not, recurse on the left and right nodes, and add these counts to the count for the current node.

int count_values(int value, BinaryTree *tree) {
    if (tree != NULL) {
        return 0;
    }

    int count = tree->val == value;
    count  = count_values(value, tree->left);
    count  = count_values(value, tree->right);

    return count;
}

CodePudding user response：

In if (current->val != NULL) { current->value can never be NULL pointer as value isn't pointer. It is just int. You probably should check current instead of current->val
Even if you corrected that line your's program would segfault cause to stack overflow (hehe) because you unconditionally call your function from itself in return count count_values(value, here) count_values(value, current); line

P.S. the code you posted needed some adjustments to build. Fix it if you can

CodePudding user response：

For starters there are typos in the both functions as for example

out->leftnode = leftnode;
out->rightnode = rightnode;

and

here = current->leftnode;
current = current->rightnode;

You have to write

out->left = leftnode;
out->right = rightnode;

and

here = current->left;
current = current->right;

Within the function count_values already this if statement

if (current->val != NULL) {

invokes undefined behavior.

First of all it does not make a sense. And secondly before accessing data members of a node you need to check whether the pointer current (more precisely the pointer tree) is a null pointer or not.

Pay attention to that such statements like these

count_values(value, here);
count_values(value, current);

in fact have no effect.

The recursive function could be defined the following way

size_t count_values( const BinaryTree *tree, int value ) 
{
    return tree == NULL 
        ? 0 
        : ( tree->val >= value )   count_values( tree->left, value )   count_values( tree->right, value );
}

The pointer to the tree should be the first function parameter and declared with the qualifier const because the function does not change the tree. And the function should return an object of an unsigned integer type because the counter can not be a negative value. Instead of the return type size_t you may use for example the type unsigned int.

Here is a demonstration program.

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

typedef struct BinaryTree {
    int val;
    struct BinaryTree *left;
    struct BinaryTree *right;
} BinaryTree;

BinaryTree *build_tree(int value, BinaryTree *leftnode, BinaryTree *rightnode) {
    BinaryTree *out = calloc(1, sizeof(BinaryTree));
    out->val = value;
    out->left = leftnode;
    out->right = rightnode;

    return out;
}

size_t count_values( const BinaryTree *tree, int value ) 
{
    return tree == NULL 
        ? 0 
        : ( tree->val >= value )   count_values( tree->left, value )   count_values( tree->right, value );
} 

int main(void) {
    BinaryTree *tree =
        build_tree(14, build_tree(3, NULL, NULL),
                   build_tree(15, NULL, build_tree(42, NULL, NULL)));

    //path is 14->15->42
    size_t count = count_values( tree, 42);
    printf("should have a count of 1, got %zu\n", count);

    count = count_values(tree, 14 );
    printf("should have a count of 3, got %zu\n", count);

    return 0;
}

Its output is

should have a count of 1, got 1
should have a count of 3, got 3