Cactus binary tree implementation in src/util/BinaryTree.c is broken

Create issue
Issue #936 closed
Roland Haas created an issue

It contains an obvious bug of the form:

int i = something;

if(i<0) {...}
else if(i>0) {...}
else if(i==0} {...}
else {do something else}

which is clearly nonsensical (this is the second half of the patch). It also triggers segfaults since it blindly recurses into NULL pointers.

The second one concerns adding elements into the tree, which always compares to the prospective subtree's parent rather than the subtree itself.

No thorn seems to use these functions right now, nor are they documented.

Keyword:

Comments (7)

  1. Roland Haas reporter
    • removed comment

    I second that. The binary tree implementation is odd even when making it work. It's interface is also awkward. I nice set of simple wrappers around std::map<,> sounds like a good idea (in particular if we can also provide Fortran wrappers since Fortran does not have any such thing right now).

  2. Erik Schnetter
    • removed comment

    I would not wrap it in C code -- I would use it from C++. It is rather easy to convert C code to C++; what is mostly necessary is to add extern "C" around scheduled functions. I don't think it is necessary to provide Fortran wrapper; it will make more sense in most cases to implement the whole algorithm in C++, and only write e.g. compute kernels in Fortran.

  3. Log in to comment