|WARNING| eal: check for invalid memory parameters

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-07-23

# DPDK Patch Review

## Summary
This patch replaces `atoi()` with a new `eal_parse_num()` helper function to improve input validation for EAL memory parameters. The change improves robustness by detecting invalid input that `atoi()` would silently accept.

---

## Errors

None found.

---

## Warnings

### 1. Missing validation for memory_size upper bound
**Location:** `eal_parse_args()`, memory_size handling

The `memory_size` parameter is parsed and multiplied by `1024 * 1024` to convert MB to bytes, but there is no upper bound check before the multiplication. If a user provides a very large value (e.g., close to `UINT_MAX`), the multiplication will overflow.

**Suggested fix:**
```c
if (args.memory_size != NULL) {
	unsigned int mb;
	if (eal_parse_num(args.memory_size, &mb) < 0) {
		EAL_LOG(ERR, "invalid memory size parameter");
		return -1;
	}

	/* Check for overflow: mb * 1024 * 1024 must fit in uint64_t */
	if (mb > (UINT64_MAX / (1024ULL * 1024ULL))) {
		EAL_LOG(ERR, "memory size parameter too large");
		return -1;
	}

	int_cfg->memory = mb;
	int_cfg->memory *= 1024ULL;
	int_cfg->memory *= 1024ULL;
}
```

### 2. Inconsistent whitespace in conditional
**Location:** Line 2248-2249

Mixed tabs and spaces in the conditional expression:
```c
if (eal_parse_num(args.memory_channels, &n) < 0 ||
    	n == 0 || n > 32) {
```

The continuation line uses spaces followed by a tab. Should use consistent indentation (double-indent with tabs for continuation).

**Suggested fix:**
```c
if (eal_parse_num(args.memory_channels, &n) < 0 ||
		n == 0 || n > 32) {
```

---

## Info

### 1. Upper bound validation added for memory_channels
The patch adds an upper bound check (`n > 32`) for memory channels that was not present in the original code. This is a good improvement, though the commit message does not mention it. Consider documenting this new validation in the commit message.

### 2. Opportunity to share validation logic
The memory_channels and memory_ranks validation both follow the same pattern (parse, check range). Consider adding a helper like `eal_parse_num_range()` to reduce duplication:

```c
static int
eal_parse_num_range(const char *str, unsigned int *val,
		    unsigned int min, unsigned int max)
{
	unsigned int n;
	
	if (eal_parse_num(str, &n) < 0 || n < min || n > max)
		return -1;
	
	*val = n;
	return 0;
}
```

This would simplify the call sites to:
```c
if (eal_parse_num_range(args.memory_channels, &n, 1, 32) < 0) {
	EAL_LOG(ERR, "invalid memory channel parameter");
	return -1;
}
```

---

## Positive Observations

1. **Correctness improvement:** The new `eal_parse_num()` function correctly handles trailing garbage, leading whitespace, negative numbers, and overflow.

2. **Error checking:** Proper validation of `errno`, `*str == '\0'`, `*endptr != '\0'`, and `n > UINT_MAX`.

3. **Style compliance:** The new function follows DPDK coding style (return type on own line, proper brace placement, explicit comparisons).

4. **Type safety:** Uses `unsigned int` instead of `int` where appropriate (memory sizes cannot be negative).