feat(storage): add feature header for multi stream feature#16170
feat(storage): add feature header for multi stream feature#16170v-pratap wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a FeatureTracker class to track and report features adopted during storage operations, specifically tracking multi-stream usage in multi-range downloads (kMultiStreamInMRD). The tracked features are encoded as a bitmask, Base64-encoded, and sent via the x-goog-storage-cpp-features metadata header. Feedback was provided on FeatureTracker::RegisterFeature to defensively check that the feature enum value is less than 32 before shifting, preventing potential undefined behavior if more features are added in the future.
| void RegisterFeature(TrackedFeature feature) { | ||
| mask_.fetch_or(1U << static_cast<std::uint32_t>(feature), | ||
| std::memory_order_relaxed); | ||
| } |
There was a problem hiding this comment.
Shifting 1U by static_cast<std::uint32_t>(feature) can result in undefined behavior if feature has a value of 32 or greater (since mask_ is a 32-bit atomic integer). To prevent potential undefined behavior as new features are added to the TrackedFeature enum in the future, we should add a safety check to ensure the shift value is strictly less than 32.
void RegisterFeature(TrackedFeature feature) {
auto const shift = static_cast<std::uint32_t>(feature);
if (shift < 32) {
mask_.fetch_or(1U << shift, std::memory_order_relaxed);
}
}References
- Prefer defensive code to prevent potential undefined behavior or contract changes in the future.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16170 +/- ##
==========================================
- Coverage 92.21% 92.20% -0.02%
==========================================
Files 2265 2268 +3
Lines 209121 209317 +196
==========================================
+ Hits 192835 192994 +159
- Misses 16286 16323 +37 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ng across all transports (googleapis#2657)
No description provided.