0005 — Winch::LoadAttachedGeometryDataFromFile tests an unsigned index for < 0, so the super-element step-back never runs

ID	0005
Class	BUG
Severity	2
Status	blocked
Models	WinchableCable
Found	2026-09-24 issue-resolution pass at 6921106, found by reading the code while reproducing CORIBO-0004
Decision needed	Model owner: confirm what the pop-back loop should do when it steps back past the first sub-element of a super-element, and whether it may step back past super-element 0

Evidence

sub_ix and super_ix are size_t & (include/fhsim_coribo/Winch.h:158-159, src/Structures/Winch.cpp:133). The loop that runs at the end of the init-states data (src/Structures/Winch.cpp:169-181) does:

PopElement(temp_states_buffer);
element = m_attached_elements.back();
sub_ix--;
if (sub_ix < 0) {
  super_ix--;
  sub_ix = super_elements[super_ix]->GetSubElements()->size() - 1;
}

sub_ix < 0 can never be true for an unsigned type. When sub_ix is 0, sub_ix-- wraps it to SIZE_MAX, super_ix is not decremented, and the caller then uses the wrapped index in GetSubElements()->at(m_num_retracted_sub) (src/Structures/SupergridWinchCable.cpp:203-205, where the caller passes m_num_retracted_sub as sub_ix). That call throws std::out_of_range.

This is shown by the code, not by a run. I have not confirmed whether the shipped example reaches this branch, because it fails earlier (CORIBO-0004).

Effect

If the pop loop steps back across a super-element boundary, initialisation either throws std::out_of_range or continues with wrong indices. Whether that happens depends on the winch angle and the contents of the init-states file.

Why this was not fixed

The obvious rewrite is if (sub_ix == 0) { --super_ix; sub_ix = size - 1; } else { --sub_ix; }. It is almost certainly what was meant, but it changes which cable elements start on the drum. That is initial-condition physics, and there is no working run to compare against (CORIBO-0004). The rewrite also needs a rule for super_ix == 0, which the current code does not define. The owner should decide both points together with CORIBO-0004.

Possible fix

Replace the decrement and test with the explicit boundary check above, and guard against super_ix == 0, or assert that it cannot happen.

Test that would prove it

A unit test that loads a short, synthetic init-states list whose pop-back crosses a super-element boundary, then checks the resulting (super_ix, sub_ix).

Risk

The code change is local, but it changes the initial element allocation whenever the branch is reached.