Guard the zero-length copy in the Span copy helpers

[Alami-Amine]

Summary

Problem

• The three Span copy helpers in src/lib/support/Span.h pass data() straight to memmove().
• An empty Span may have a null data() — TLVReader::GetDataPtr returns one for every zero-length string — and passing a null pointer to memmove() is undefined even at zero length (C17 7.24.1p2), which -fsanitize=undefined reports as null pointer passed as argument 2, which is declared to never be null.

Solution

• Skip the copy when nothing is copied.
• CopyCharSpanToMutableCharSpanWithTruncation checks the clamped size rather than source emptiness: it truncates instead of failing, so size_to_copy is zero for an empty destination even when the source is not empty. Guarding on source emptiness there would leave the null destination case unguarded.

Caveats

• No behavioural change outside a sanitizer build.

Testing

• Added TestCopySpanToMutableSpanFromNullSource, TestCopyCharSpanToMutableCharSpanFromNullSource and TestCopyCharSpanToMutableCharSpanWithTruncationFromNullSource to src/lib/support/tests/TestSpan.cpp, covering a null source into both a sized and a default-constructed destination, plus a non-empty source into a zero-size destination for the truncation helper.
• Verified against linux-x64-tests-clang-ubsan with UBSAN_OPTIONS=halt_on_error=1: without the change the suite exits 1 reporting Span.h:406; with it, 14 tests pass and no reports are emitted.
• The truncation case was checked to discriminate the guard: replacing size_to_copy != 0 with !span_to_copy.empty() reports null pointer passed as argument 1 and exits 1, while the null-source tests alone stay green.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 8022fe60-7b28-48de-96f9-49a565690ad1

📥 Commits

Reviewing files that changed from the base of the PR and between f20c00f and 40f3786.

📒 Files selected for processing (3)

• src/lib/support/Span.h
• src/lib/support/tests/BUILD.gn
• src/lib/support/tests/TestSpan.cpp

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The span copy helpers now skip memmove when the source or computed copy size is empty. Tests cover null source spans, empty destinations, unchanged buffers, and truncation into zero-size destinations. The test target also exposes the required local test-macro dependency.

Suggested reviewers: ksperling-apple

Merge Risk: ⚪ Minimal · up to 40f37

Empty span copies now avoid undefined behavior while preserving destination contents; the change is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: guarding zero-length copies in the Span copy helpers.
Description check	✅ Passed	The description is directly related to the changeset and explains the problem, solution, tests, and sanitizer results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

---

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.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.87%. Comparing base (51ff55e) to head (40f3786).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #74061      +/-   ##
==========================================
- Coverage   57.34%   56.87%   -0.47%     
==========================================
  Files        1943     1985      +42     
  Lines      128154   129319    +1165     
  Branches    14605    15044     +439     
==========================================
+ Hits        73491    73556      +65     
- Misses      54663    55763    +1100     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ForemanZack-CableLabs]

Looks good to me and catches that issue