|WARNING| fbarray: check fbarray lock downgrade failure

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Patch Summary
File: `lib/eal/common/eal_common_fbarray.c`
Change: Add error checking for `eal_file_lock()` downgrade operation in `rte_fbarray_destroy()` error path.

---

## Errors

None found.

---

## Warnings

### 1. Log message format inconsistency
**File:** `lib/eal/common/eal_common_fbarray.c:1026`

The log message uses `rte_strerror(rte_errno)`, but most EAL logging in this file and elsewhere uses errno value directly with `%s` and `strerror()`, or relies on the log message alone without errno details.

**Current:**
```c
EAL_LOG(DEBUG, "Cannot restore fbarray shared lock: %s",
    rte_strerror(rte_errno));
```

**More consistent pattern (used elsewhere in DPDK):**
```c
EAL_LOG(DEBUG, "Cannot downgrade fbarray lock to shared");
```

The error is already non-fatal (we're logging at DEBUG level and preserving the original `rte_errno` from the unlink failure), so including errno details may not add value. However, if errno is desired, the current approach is acceptable.

---

## Info

### 1. Error handling approach is sound
The patch correctly:
- Checks the return value of `eal_file_lock()` which was previously ignored
- Logs the failure at an appropriate level (DEBUG - this is a cleanup failure in an error path)
- Preserves the original `rte_errno` from the `unlink()` failure, which is the primary error being reported to the caller
- Does not change control flow (still proceeds to `goto out` with `ret = -1`)

This is the right approach for an error-path cleanup failure: log it, but don't override the original error.

### 2. Comment accuracy
The existing comment says "drop it to shared" which accurately describes the lock downgrade operation. The new code properly handles the case where this downgrade fails.

---

## Summary

**Verdict:** The patch is correct and improves error handling.

The only minor suggestion is consistency in log message formatting (whether to include errno details or not), but the current approach is acceptable. The core logic - checking the previously-ignored return value and logging failures while preserving the original error - is sound.