fix: remove reference in time frame builder

[osbornjd]

The TPC event combining jobs occasionally seg fault. Valgrind claimed a problem at QA histogram filling, which points back to this reference being used. Removing the reference causes valgrind to no longer indicate an invalid read, and testing on one of the files that identified this the segfault no longer occurs. This will cost some CPU performance decrease for better stability

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work for users)
• Requiring change in macros repository (Please provide links to the macros pull request in the last section)
• I am a member of GitHub organization of sPHENIX Collaboration, EIC, or ECCE (contact Chris Pinkenburg to join)

What kind of change does this PR introduce? (Bug fix, feature, ...)

TODOs (if applicable)

Links to other PRs in macros and calibration repositories (if applicable)

TPC Time Frame Builder: Dangling Reference Removal

Motivation and Context

This PR fixes occasional segmentation faults encountered during TPC event combining jobs. Valgrind identified an invalid memory read originating from QA histogram filling in the time frame builder. Root cause analysis traced this to a dangling reference: pkt_length was aliasing data_buffer[0], but the underlying deque was subsequently modified via pop_front() and erase() operations, invalidating the reference before it was used in QA histogram filling.

Key Changes

• TpcTimeFrameBuilder.cc (line 716): Changed const reference to const value copy
  • Before: const uint16_t& pkt_length = data_buffer[0];
  • After: const uint16_t pkt_length = data_buffer[0];
  • Eliminates the dangling reference by creating a value copy instead of an alias to a container element
  • Container modifications (pop_front() at lines 697/709/724, erase() at line 749) can no longer invalidate the pkt_length variable
  • All downstream usage of pkt_length remains semantically identical; no logic changes

Validation and Risk Assessment

• Stability improvement: Valgrind no longer reports the invalid memory read; a file that previously triggered a segmentation fault now processes successfully
• Performance trade-off: Marginal CPU overhead from copying a uint16_t value is acceptable in exchange for eliminating undefined behavior
• Risk areas:
  • Performance: Minimal overhead from value copy instead of reference binding (uint16_t = 2 bytes)
  • No reconstruction behavior changes: All QA output and physics logic remain unchanged; only memory safety is improved
  • Thread-safety: No new thread-safety concerns introduced

Notes

• AI-generated analysis may contain inaccuracies; please review the actual code logic for validation
• This is a surgical fix addressing a specific memory safety issue without architectural changes

[coderabbitai]

📝 Walkthrough

Walkthrough

Modified the pkt_length variable declaration in two FEE-processing code paths within TpcTimeFrameBuilder.cc from a const reference to a const value, eliminating potential dangling-reference issues when data_buffer is modified.

Changes

Cohort / File(s)	Summary
Reference-to-Value Conversion offline/framework/fun4allraw/TpcTimeFrameBuilder.cc	Changed pkt_length from const uint16_t& to const uint16_t in two FEE-processing paths to prevent dangling references when data_buffer undergoes modifications (pop/erase operations). Usage logic remains unchanged.

✨ Finishing touches

• 📝 Generate docstrings

---

No actionable comments were generated in the recent review. 🎉

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sphenix-jenkins-ci]

Build & test report

Report for commit 0a41d07cf2e4fcfaa240632a2a7b4be646bbcfdc:

• builds and tests overall are SUCCESS.
• Build with configuration of alma9.2-gcc-14.2.0 / clang is SUCCESS, 📊clang report (full)/(new), build log
• Build with configuration of alma9.2-gcc-14.2.0 / new is SUCCESS, 📊Compiler report (full)/(new), build log
  • Generating DST and readback: build is SUCCESS
  • Calorimeter QA: build is SUCCESS, 📊 trends
    • 📊 QA-calorimeter for e- at p_T=4GeV : combined Chi2/nDoF = 27.45 / 72, and combined p-Value = 1
    • 📊 QA-calorimeter for pi+ at p_T=30GeV : combined Chi2/nDoF = 41.6183 / 72, and combined p-Value = 0.998442
    • 📊 QA-calorimetric-jet for e- at p_T=4GeV : combined Chi2/nDoF = 0.4559 / 42, and combined p-Value = 1
    • 📊 QA-calorimetric-jet for pi+ at p_T=30GeV : combined Chi2/nDoF = 14.2531 / 42, and combined p-Value = 0.999981
  • Tracking QA with Distortions: build is SUCCESS, 📊 trends
    • 📊 QA-distortions
  • Tracking QA at low occupancy: build is SUCCESS, 📊 trends
    • 📊 QA-Intt : combined Chi2/nDoF = -0 / 72, and combined p-Value = 1
    • 📊 QA-Micromegas : combined Chi2/nDoF = -0 / 20, and combined p-Value = 1
    • 📊 QA-Mvtx : combined Chi2/nDoF = -0 / 54, and combined p-Value = 1
    • 📊 QA-Tpc : combined Chi2/nDoF = -0 / 56, and combined p-Value = 1
    • 📊 QA-tracking : combined Chi2/nDoF = -0 / 56, and combined p-Value = 1
    • 📊 QA-vertexing : combined Chi2/nDoF = 30 / 112, and combined p-Value = 1
  • Tracking QA for Pythia D0-jet: build is SUCCESS, 📊 trends
    • 📊 QA-Intt : combined Chi2/nDoF = -0 / 72, and combined p-Value = 1
    • 📊 QA-Micromegas : combined Chi2/nDoF = -0 / 20, and combined p-Value = 1
    • 📊 QA-Mvtx : combined Chi2/nDoF = -0 / 54, and combined p-Value = 1
    • 📊 QA-Tpc : combined Chi2/nDoF = 8.29825e-12 / 56, and combined p-Value = 1
    • 📊 QA-tracking : combined Chi2/nDoF = 5.52413 / 50, and combined p-Value = 1
    • 📊 QA-vertexing : combined Chi2/nDoF = 30 / 112, and combined p-Value = 1
  • Tracking QA from run2pp 53877: build is SUCCESS, 📊 trends
    • 📊 QA-intt : combined Chi2/nDoF = -0 / 18, and combined p-Value = 1
    • 📊 QA-micromegas : combined Chi2/nDoF = -0 / 6, and combined p-Value = 1
    • 📊 QA-mvtx : combined Chi2/nDoF = -0 / 18, and combined p-Value = 1
    • 📊 QA-siliconseeds : combined Chi2/nDoF = -0 / 20, and combined p-Value = 1
    • 📊 QA-tpc : combined Chi2/nDoF = -0 / 48, and combined p-Value = 1
    • 📊 QA-tpcseeds : combined Chi2/nDoF = -0 / 28, and combined p-Value = 1
    • 📊 QA-trackfit : combined Chi2/nDoF = -0 / 32, and combined p-Value = 1
  • system alma9.2-gcc-14.2.0, build new: run the default sPHENIX macro: build is SUCCESS, output, trends 📊
  • system alma9.2-gcc-14.2.0, build new: run the default CaloProduction/Fun4All_Year2.C macro: build is SUCCESS, output, trends 📊
  • system alma9.2-gcc-14.2.0, build new: run the default StreamingProduction/Fun4All_Stream_Combiner.C macro: build is SUCCESS, output, trends 📊
  • system alma9.2-gcc-14.2.0, build new: Valgrind test of CaloProduction/Fun4All_Year2.C: build is SUCCESS, 📊valgrind report, trends 📊
  • system alma9.2-gcc-14.2.0, build new: Valgrind test of StreamingProduction/Fun4All_Stream_Combiner.C: build is SUCCESS, 📊valgrind report, trends 📊
  • system alma9.2-gcc-14.2.0, build new: run the overlap check for sPHENIX macro: build is SUCCESS, output
  • system alma9.2-gcc-14.2.0, build new: Valgrind test: build is SUCCESS, 📊valgrind report, trends 📊
• Build with configuration of alma9.2-gcc-14.2.0 / scan is SUCCESS, 📊scan-build report (full)/(new), build log
• clang-tidy is UNSTABLE, 📊clang-tidy report (full)/(new)
• cpp-check is SUCCESS, 📊cppcheck report (full)/(new)

---

Automatically generated by sPHENIX Jenkins continuous integration
           

[osbornjd]

The clang tidy warnings are benign