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

Issue #936 closed

Roland Haas created an issue 2012-05-29

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)

Erik Schnetter
- removed comment
I suggest to remove this code, and to recommend to use std::map<,> instead.
- 2012-05-29T18:05:50+00:00
Roland Haas reporter
- changed status to open
- removed comment
- 2012-05-29T18:22:47+00:00
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).
- 2012-05-29T18:26:21+00:00
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.
- 2012-05-29T21:40:18+00:00
Erik Schnetter
- changed status to open
- removed comment
Removing the binary tree implementation and interface is approved.
- 2012-06-11T10:31:33+00:00
Erik Schnetter
- changed status to resolved
- removed comment
Done.
- 2012-06-15T15:17:45+00:00
Roland Haas reporter
- edited description
- changed status to closed
- 2019-02-21T19:55:12+00:00
Log in to comment