> On Mon, Apr 16, 2012 at 10:08, Christopher Sean Morrison <brlcad@...> wrote:
>> On Apr 16, 2012, at 10:56 AM, Tom Browder wrote:
>>>  Does that mean the tree pointer itself should have been set to
>>> zero after the free?
>> Possibly, but usually the "dead" tree nodes are removed.
> After some more investigating I've done the following (trying to take
> the "first, do no harm" approach):
> 1. In db_tree.c in the db_free_tree function I've left this line in (1469):
> tp->magic = (uint32_t)-3; /* special bad flag */
> and, at the end of the function, inserted these lines:
> /* reset magic after recursion */
> tp->magic = RT_TREE_MAGIC;
> That seems like a conservative way to fix the problem of the smash
> flag left in the tree, and all the crashes on g-nff and friends are no
> longer happening when calling the db_free_tree function in the catch
> block or anywhere else.
> I have checked the change in for the db_tree.c as an immediate fix,
> but I would appreciate some other eyeballs on it.
In determining what the magic value should be, it's actually pretty simple. The magic number is supposed to reflect the usability of an object. If it's got valid memory, is initialized, and is usable, then the structure should be set to whatever *_MAGIC symbol corresponds for that structure type. If it's not valid, it's supposed to be set to something/anything else. Common "invalid" values are -1, -3 (as seen), and 0.
So to magic should only be set to RT_TREE_MAGIC if that tree pointer is indeed still usable. The fact that it was just set to -3 would usually imply (I haven't looked at code just yet) that it's either no longer valid or it will very soon be invalid/free'd. Setting it to RT_TREE_MAGIC might make a bomb check pass .. but those checks are in place to catch a whole variety of coding bugs. The magic number isn't usually the cause, so that doesn't seem like the right fix at a glance... It's on my list to look into after I finish up with the Coverity defects.