|WARNING| [v2] bus/fslmc: convert queue storage macros to inline functions

[dpdklab at iol.unh.edu]

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

## Patch Summary
This patch converts queue storage allocation/free macros to static inline functions to avoid shadow warnings with `-Wshadow`.

---

## Errors

### 1. Resource leak on partial allocation failure
**File:** `drivers/bus/fslmc/portal/dpaa2_hw_pvt.h`
**Function:** `dpaa2_queue_storage_alloc()`

When allocation fails in the middle of the loop, previously allocated storage is not freed before returning the error.

```c
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]) {
		ret = -ENOBUFS;
		break;  /* BUG: leaks q->q_storage[0..i-1] */
	}

	ret = dpaa2_alloc_dq_storage(q->q_storage[i]);
	if (ret)
		break;  /* BUG: leaks q->q_storage[0..i] */
}

return ret;
```

**Fix:** Clean up all previously allocated storage on 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 != 0)
			goto cleanup;
	}

	return 0;

cleanup:
	/* Free all successfully allocated entries */
	dpaa2_queue_storage_free(q, i + 1);
	return ret;
}
```

---

## Warnings

### 1. Missing function declarations in header
**File:** `drivers/bus/fslmc/portal/dpaa2_hw_pvt.h`

The patch adds forward declarations for `dpaa2_alloc_dq_storage()` and `dpaa2_free_dq_storage()` but these functions are likely defined elsewhere. Verify that:
- These declarations match the actual function signatures
- The functions are actually exported/accessible to callers of this header

If these are internal functions not meant to be called directly, consider keeping them out of the header or marking them appropriately.

---

## Info

### 1. Implicit comparison style
**File:** `drivers/bus/fslmc/portal/dpaa2_hw_pvt.h`

The code uses explicit NULL comparison in the free function (`if (!q)`) but implicit comparison in the allocation function (`if (!q->q_storage[i])`). For consistency, both should use explicit comparisons:

```c
/* Current - mixed style */
if (!q)
	return;

if (!q->q_storage[i]) {

/* Preferred - consistent explicit style */
if (q == NULL)
	return;

if (q->q_storage[i] == NULL) {
```

However, this is a minor style preference and both forms appear in the original macros, so maintaining the current style is acceptable.

### 2. Function documentation
Consider adding Doxygen comments for the new inline functions, especially since they are in a header file:

```c
/**
 * Allocate queue storage for DPAA2 queue.
 *
 * @param q
 *   Pointer to DPAA2 queue structure
 * @param num
 *   Number of storage entries to allocate
 * @return
 *   0 on success, negative errno on failure
 */
static inline int
dpaa2_queue_storage_alloc(struct dpaa2_queue *q, int num)
```

---

## Summary

The patch correctly converts macros to inline functions, which is a good improvement for type safety and avoiding scope issues. However, it introduces a **critical resource leak bug** on the error path that must be fixed before merging.

**Required changes:**
1. Add error path cleanup in `dpaa2_queue_storage_alloc()` to free partially allocated storage

**Recommended changes:**
1. Verify forward declarations match actual function definitions
2. Consider adding function documentation