Skip to content

Fix MinHeap::IsValid() skipping root's children#144

Merged
davideberly merged 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/minheap-isvalid
Feb 27, 2026
Merged

Fix MinHeap::IsValid() skipping root's children#144
davideberly merged 1 commit intodavideberly:masterfrom
CodeReclaimers:fix/minheap-isvalid

Conversation

@CodeReclaimers
Copy link
Contributor

Summary

  • IsValid() uses if (parent > 0) but should use if (child > 0). When child is 1 or 2, parent is 0, so parent > 0 is false and the root's children are never validated against the root.
Test program
// Test for issue 10.8: MinHeap::IsValid() skips validation of root's children.
//
// The bug: IsValid() uses `if (parent > 0)` but should use `if (child > 0)`.
// When child is 1 or 2, parent is 0, and `parent > 0` is false, so the
// parent-child relationship at the root is never checked.
//
// This test manually corrupts a heap so that a child of the root has a
// smaller value than the root, then checks whether IsValid() detects it.

#include <Mathematics/MinHeap.h>
#include <cstdlib>
#include <iostream>

using namespace gte;

int main()
{
    // Build a valid 3-element min-heap: values 1, 2, 3.
    MinHeap<int, int> heap(4);
    heap.Insert(0, 1);  // root: value 1
    heap.Insert(1, 2);  // left child: value 2
    heap.Insert(2, 3);  // right child: value 3

    if (!heap.IsValid())
    {
        std::cerr << "FAIL: valid heap reported as invalid" << std::endl;
        return EXIT_FAILURE;
    }

    MinHeap<int, int> heap2(4);
    auto* r0 = heap2.Insert(0, 10);  // root: value 10
    auto* r1 = heap2.Insert(1, 20);  // left child: value 20
    auto* r2 = heap2.Insert(2, 30);  // right child: value 30

    // Heap is valid: 10 <= 20 and 10 <= 30.
    if (!heap2.IsValid())
    {
        std::cerr << "FAIL: valid heap2 reported as invalid" << std::endl;
        return EXIT_FAILURE;
    }

    // Corrupt the heap: set root's value to 50, making it larger than
    // both children. We write directly to the record to bypass Update().
    r0->value = 50;

    // Now the heap property is violated: root(50) > left(20) and root(50) > right(30).
    // IsValid() should return false.
    if (heap2.IsValid())
    {
        std::cerr << "FAIL: IsValid() returned true for corrupted heap "
                  << "(root=50, children=20,30)" << std::endl;
        return EXIT_FAILURE;
    }

    std::cout << "PASS: IsValid() correctly detects corruption at root's children"
              << std::endl;
    return EXIT_SUCCESS;
}

Before fix: FAIL: IsValid() returned true for corrupted heap (root=50, children=20,30)
After fix: PASS: IsValid() correctly detects corruption at root's children

IsValid() used `if (parent > 0)` but should use `if (child > 0)`.
When child is 1 or 2, parent is 0, so `parent > 0` is false and
the root's children are never checked against the root.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@davideberly davideberly merged commit 98fdfcb into davideberly:master Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants