|WARNING| [v2] bus/fslmc: convert queue storage macros to inline functions
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Sun Jun 28 18:09:10 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/166373
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-28
# DPDK Patch Review
## Summary
This patch converts two macros (`dpaa2_queue_storage_alloc` and `dpaa2_queue_storage_free`) to static inline functions to avoid shadow warnings when `-Wshadow` is enabled.
---
## Errors
### 1. Resource leak on error path
In `dpaa2_queue_storage_alloc()`, when `dpaa2_alloc_dq_storage()` fails on iteration `i`, the previously allocated `q->q_storage[i]` from `rte_zmalloc()` is leaked because the loop breaks without freeing it.
**Fix:** Free `q->q_storage[i]` before breaking when `dpaa2_alloc_dq_storage()` fails:
```c
ret = dpaa2_alloc_dq_storage(q->q_storage[i]);
if (ret) {
rte_free(q->q_storage[i]);
q->q_storage[i] = NULL;
break;
}
```
### 2. Partial cleanup on allocation failure
When `dpaa2_queue_storage_alloc()` fails partway through, it does not clean up previously successful allocations (`q->q_storage[0]` through `q->q_storage[i-1]`). The caller is left with a partially allocated queue storage array.
**Fix:** Add cleanup of previously allocated entries before returning error:
```c
static inline int
dpaa2_queue_storage_alloc(struct dpaa2_queue *q, int num)
{
int ret = 0, i;
for (i = 0; i < num; i++) {
q->q_storage[i] = rte_zmalloc(NULL,
sizeof(struct queue_storage_info_t),
RTE_CACHE_LINE_SIZE);
if (q->q_storage[i] == NULL) {
ret = -ENOBUFS;
goto cleanup;
}
ret = dpaa2_alloc_dq_storage(q->q_storage[i]);
if (ret) {
rte_free(q->q_storage[i]);
q->q_storage[i] = NULL;
goto cleanup;
}
}
return 0;
cleanup:
dpaa2_queue_storage_free(q, i);
return ret;
}
```
---
## Warnings
### 1. Function declarations should be in header
The patch adds forward declarations for `dpaa2_alloc_dq_storage()` and `dpaa2_free_dq_storage()` directly in the header where they're used. If these functions are defined elsewhere, their declarations should appear near the top of the header with other function prototypes, not inline with the static inline definitions.
**Suggestion:** Move the declarations to the appropriate location in the header (typically after struct definitions but before inline function definitions).
---
## Info
### 1. Explicit NULL comparison style
The patch correctly converts the implicit truthiness checks from the macro form to explicit comparisons (`if (!q)`, `if (!q->q_storage[i])`), which complies with DPDK coding standards.
### 2. Return value handling improvement
Converting from a statement expression macro to a proper function makes the return value handling clearer and type-safe.
---
## Correctness Check Passed
- No use-after-free issues (besides the leak)
- No race conditions (single-threaded allocation pattern)
- No missing error checks on `rte_zmalloc()` (correctly handled)
- Proper NULL pointer checks before dereference
- No forbidden tokens
- No volatile misuse
- No atomic/threading issues
More information about the test-report
mailing list