Skip to content

[treereader] Correctly treat empty trees with range loop iteration#16257

Merged
dpiparo merged 1 commit intoroot-project:masterfrom
dpiparo:fix_16249
Aug 18, 2024
Merged

[treereader] Correctly treat empty trees with range loop iteration#16257
dpiparo merged 1 commit intoroot-project:masterfrom
dpiparo:fix_16249

Conversation

@dpiparo
Copy link
Member

@dpiparo dpiparo commented Aug 16, 2024

instead of allowing a spurious iteration with zero entries.

Fixes #16249

@dpiparo dpiparo self-assigned this Aug 16, 2024
@dpiparo dpiparo requested a review from pcanal as a code owner August 16, 2024 14:23
@github-actions
Copy link

github-actions bot commented Aug 16, 2024

Test Results

    13 files      13 suites   2d 21h 30m 35s ⏱️
 3 028 tests  3 028 ✅ 0 💤 0 ❌
33 838 runs  33 838 ✅ 0 💤 0 ❌

Results for commit fed262b.

♻️ This comment has been updated with latest results.

Comment on lines 96 to 102
if (fReader->GetTree()->GetEntries() == 0 && fEntry == 0 && !lhs.IsValid()) {
return true;
}
if (lhs.fReader->GetTree()->GetEntries() == 0 && lhs.fEntry == 0 && !IsValid()) {
return true;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (fReader->GetTree()->GetEntries() == 0 && fEntry == 0 && !lhs.IsValid()) {
return true;
}
if (lhs.fReader->GetTree()->GetEntries() == 0 && lhs.fEntry == 0 && !IsValid()) {
return true;
}
if (fReader->GetTree()->GetEntriesFast() == 0 && fEntry == 0 && !lhs.IsValid()) {
return true;
}
if (lhs.fReader->GetTree()->GetEntriesFast() == 0 && lhs.fEntry == 0 && !IsValid()) {
return true;
}

If LoadTree(0) has been called and there is really zero elements in the chain then GetEntriesFast will be zero.
If there is code path where we can get here and LoadTree(0) has not been called I suspect we could call it safely here (but hopefully we don't have this case).

@dpiparo
Copy link
Member Author

dpiparo commented Aug 17, 2024

all comments addressed, code updated, tests restarted.

instead of allowing a spurious iteration with zero entries.

Fixes root-project#16249
@dpiparo dpiparo merged commit cc65453 into root-project:master Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Iterating with a range for does one extra iteration

2 participants