|WARNING| eal: fix C++ builds when stdatomics is set

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Thu Sep 24 17:06:11 CEST 2026


Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/169993

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-24

# Review of DPDK Patch: EAL C++ Build Fix for Stdatomics

## Summary
This patch addresses a build failure when DPDK is configured with `RTE_ENABLE_STDATOMIC=true` and C++ code includes DPDK headers. The fix splits the stdatomic implementation into separate C and C++ paths.

---

## ERRORS

None identified. The patch is a build fix that resolves a genuine compatibility issue between C11 `_Atomic` and C++ atomic implementations.

---

## WARNINGS

### 1. Missing Release Notes Entry (Warning)

**Issue:** The patch fixes a significant bug (build failure in C++) but does not update release notes.

**Why it matters:** This is a user-visible bug fix that affects C++ applications using DPDK with stdatomics enabled. Users need to know the issue is resolved.

**Fix:** Add an entry to `doc/guides/rel_notes/release_26_11.rst` (or current release notes file):

```rst
* **Fixed C++ build failures with stdatomic support.**

  Resolved compilation errors when building C++ applications with DPDK
  configured to use standard C11 atomics (``RTE_ENABLE_STDATOMIC``).
  C++ now uses the GCC builtin atomics path while maintaining the same
  API surface.
```

### 2. Potentially Confusing Comment Wording (Warning)

**Issue:** The comment "keep the builtin-based implementation for C++ and continue to expose the existing rte_atomic_* API surface" could be clearer about why this is necessary.

**Suggested improvement:**
```c
/* C++ cannot use the C11 _Atomic type specifier that DPDK public headers
 * rely on (GCC does not support _Atomic(T) in C++ mode, and C++17 std::atomic
 * is incompatible with extern "C" linkage and anonymous unions).
 * Use GCC builtin atomics instead, which work identically in C and C++.
 */
```

---

## INFO

### 1. Code Structure (Info)

The three-way split (non-stdatomic / C stdatomic / C++ builtin) is a pragmatic solution. An alternative would be to use C++20/23 atomic features when available, but the current approach of using builtins for C++ is simpler and maintains API compatibility.

### 2. Testing Recommendation (Info)

Ensure the patch is tested with:
- C builds with `RTE_ENABLE_STDATOMIC=true`
- C++ builds with `RTE_ENABLE_STDATOMIC=true` (the failing case this fixes)
- C++ builds with `RTE_ENABLE_STDATOMIC=false` (should be unchanged)

The patch description indicates testing was done (build no longer fails), which is good.

---

## CORRECTNESS VERIFICATION

### Resource Management
 No resource allocation/deallocation in this patch

### API Consistency
 The C++ path provides identical macro definitions to the non-stdatomic path
 All `rte_atomic_*` and `rte_memory_order_*` symbols are defined in both paths
 Function signatures match between paths

### Synchronization
 No shared state introduced
 No race conditions possible (preprocessor-only changes)

### Style Compliance
 No trailing whitespace
 Consistent indentation (tabs)
 No forbidden tokens
 Proper header guard structure maintained

---

## OVERALL ASSESSMENT

**Verdict:** The patch is technically correct and solves a real build failure. The only actionable item is adding a release notes entry. The code is well-structured and maintains API compatibility across all build configurations.

**Recommendation:** Accept after adding release notes entry.


More information about the test-report mailing list