|WARNING| eal: fix pointer add/sub macros for large offsets values

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Fri Sep 11 19:45:09 CEST 2026


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

_AI Code Review_

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

# Review of DPDK Patch: eal: fix pointer add/sub macros for large offsets values

## Summary

This patch modifies `RTE_PTR_ADD` and `RTE_PTR_SUB` macros to add an intermediate `uintptr_t` cast before the final `void *` cast. The stated motivation is to fix errors on 32-bit builds when the offset value is larger than `uintptr_t`.

## Errors

### 1. Integer overflow in arithmetic is not prevented by additional cast

**Problem:** The patch adds `(void *)(uintptr_t)` casting but does not address the actual overflow concern described in the commit message.

**Why it matters:** On a 32-bit system where `uintptr_t` is 32 bits, if `x` is a 64-bit value (e.g., `uint64_t` or `size_t` on 64-bit), the expression `(uintptr_t)(ptr) + (x)` performs the addition at the width of the wider operand. However, if the *result* exceeds 32 bits, the outer `(uintptr_t)` cast truncates it, which is the same behavior as before the patch. The double cast `(void *)(uintptr_t)(...)` does not widen the arithmetic or prevent truncation.

**What the patch actually changes:** The additional `(uintptr_t)` cast between the arithmetic and the `void *` cast makes the cast sequence explicit but does not fix an overflow. If the arithmetic result is 33+ bits and then cast to 32-bit `uintptr_t`, upper bits are lost regardless of how many casts follow.

**Expected behavior:** If the goal is to support offsets larger than `uintptr_t` on 32-bit builds, the arithmetic itself must be performed at a wider width (e.g., `uint64_t`) before truncation to `uintptr_t`. The current patch does not do this.

**Clarification needed:** The commit message claims "if the value being added or subtracted is larger than uintptr_t, then we get errors when casting back to void * type." This suggests a compiler warning or error, not a runtime correctness issue. If the issue is a compiler diagnostic about casting a large integer constant to pointer, that would be a different problem. Please verify:
- What specific error/warning is being fixed?
- On which compiler and architecture?
- Is this a compile-time diagnostic or a runtime overflow concern?

If the issue is a compiler warning about casting a 64-bit value directly to `void *` on 32-bit, then the patch may suppress the warning but does not make the arithmetic correct for large offsets.

**Suggested action:** Clarify the actual problem being solved. If the intent is to allow offsets representable in 64 bits on 32-bit systems, the arithmetic must be done in a wider type. If the intent is only to suppress a compiler warning when a large compile-time constant is used, document that limitation.

---

## Warnings

### 1. Commit message does not explain the specific failure mode

**Issue:** The message states "we get errors when casting back to void * type" but does not specify what errors (compiler warning? build failure? which compiler version/flags?), making it hard to verify the fix is correct.

**Suggested improvement:** Include the compiler error message or warning text that motivated the change, and the build configuration (32-bit x86, ARM, compiler version) where it occurs.

---

## Info

### 1. Inconsistent spacing in cast

**Current:**
```c
((void*)((uintptr_t)(ptr) + (x)))   // original, no space after void*
((void *)(uintptr_t)((uintptr_t)(ptr) + (x)))   // patched
```

**Observation:** The original macro had `(void*)` without a space; the patch changes to `(void *)` with a space, which is the DPDK preferred style. This is good, but the change is unrelated to the stated fix. If this is intentional style cleanup, it's acceptable, but the commit message doesn't mention it.

---

## Additional Questions for Verification

1. **What is the actual error message** this patch fixes?
2. **Does the patch introduce undefined behavior** by allowing pointer arithmetic to wrap on 32-bit when `x` is a 64-bit offset that doesn't fit in the address space?
3. **Are there any existing uses** of `RTE_PTR_ADD`/`RTE_PTR_SUB` with offsets larger than `UINTPTR_MAX` that this is intended to support? If so, those are likely bugs (attempting to compute an address outside the addressable range).

---

## Conclusion

The patch does not appear to fix an integer overflow in the arithmetic itself--it only adds an intermediate cast that makes the truncation explicit. If the goal is to suppress a specific compiler warning about pointer-to-integer cast width on 32-bit builds, the patch may achieve that, but the commit message overstates what is being fixed. The arithmetic will still overflow/truncate if `x` exceeds the address space width.

**Recommendation:** Request clarification on the specific compiler error and the intended use case for offsets larger than `uintptr_t`. If the patch is meant only to silence a compiler warning, that should be stated explicitly.


More information about the test-report mailing list