Skip to content

Conversation

@ferdymercury
Copy link
Collaborator

@ferdymercury ferdymercury commented Jun 25, 2024

This Pull request:

Changes or fixes:

nullptr access when branch is not found

Fixes https://its.cern.ch/jira/browse/ROOT-8842 by @jpivarski

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

@ferdymercury ferdymercury requested a review from pcanal as a code owner June 25, 2024 13:20
@ferdymercury

This comment was marked as outdated.

@github-actions
Copy link

github-actions bot commented Jun 25, 2024

Test Results

    20 files      20 suites   3d 6h 29m 23s ⏱️
 3 780 tests  3 780 ✅ 0 💤 0 ❌
73 854 runs  73 854 ✅ 0 💤 0 ❌

Results for commit 4f04fe5.

♻️ This comment has been updated with latest results.

@dpiparo
Copy link
Member

dpiparo commented Jun 26, 2024

Thanks for this improvement. I propose the following way forward, if you agree:

  1. A small unit test is added for this fix
  2. We wait for @pcanal to approve (with a test, I would be ready to approve and to pick up the rest of the fixing, but I prefer to have his opinion on this)
  3. We promote the old JIRA item as a new GH issue specific to the residual issue

Does it make sense?

@ferdymercury

This comment was marked as outdated.

@ferdymercury ferdymercury marked this pull request as draft December 4, 2024 17:26
@ferdymercury ferdymercury force-pushed the reader branch 2 times, most recently from b0f1804 to cfc520f Compare June 30, 2025 16:20
@ferdymercury ferdymercury force-pushed the reader branch 3 times, most recently from 28b51cd to de6fd05 Compare June 30, 2025 16:25
@ferdymercury ferdymercury marked this pull request as ready for review September 16, 2025 10:10
@ferdymercury ferdymercury requested a review from pcanal September 16, 2025 10:10
@ferdymercury
Copy link
Collaborator Author

The issue seems finally fixed now (tried locally).

Side note, the equivalent function in TTreeReader(NotFast) has:

if (fProxiesSet) {
      Error("RegisterValueReader",
            "Error registering reader for %s: TTreeReaderValue/Array objects must be created before the call to Next() "
            "/ SetEntry() / SetLocalEntry(), or after TTreeReader::Restart()!",
            reader->GetBranchName());
      return false;
   }

@ferdymercury ferdymercury force-pushed the reader branch 2 times, most recently from 30daea9 to 08d201f Compare September 17, 2025 23:43
@ferdymercury ferdymercury added this to the 6.38.00 milestone Oct 6, 2025
@ferdymercury ferdymercury requested a review from hageboeck October 8, 2025 15:13
Copy link
Member

@dpiparo dpiparo left a comment

Choose a reason for hiding this comment

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

Let's see how the builds go after the rebase to the current main branch. If the builds are green, it's just a matter of removing the commented code (I can do that, no worries) and then for me the code is ready to go.

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.

4 participants