|WARNING| [v3,2/2] eal: add destructor to unregister tailq on unload

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-10

# DPDK Patch Review

## PATCH 1/2: eal: fix off by one in in tailq name init

### Correctness

**No issues found.**

The fix is correct. `strlcpy(dst, src, size)` copies up to `size-1` characters and always null-terminates, so passing `sizeof(head->name)` instead of `sizeof(head->name) - 1` is the proper usage. The old code artificially limited strings to 30 bytes when the buffer is 32 bytes.

### Style

**No issues found.**

---

## PATCH 2/2: eal: add destructor to unregister tailq on unload

### Errors

#### 1. Missing error check in `rte_eal_tailq_update()`

```c
if (t->head == NULL) {
    /* slot reserved by an earlier load -- reuse it */
    t->head = rte_eal_tailq_lookup(t->name);
}
```

`rte_eal_tailq_lookup()` can return `NULL` if the name is not found. The code does not check the result of this second lookup, so `t->head` could remain `NULL` after the fallback path, leading to a NULL pointer dereference when the caller uses `t->head`.

**Suggested fix:**

```c
if (t->head == NULL) {
    /* slot reserved by an earlier load -- reuse it */
    t->head = rte_eal_tailq_lookup(t->name);
    if (t->head == NULL) {
        EAL_LOG(ERR, "Cannot find TAILQ entry %s", t->name);
        return -1;
    }
}
```

#### 2. Missing export macro in header

The new `rte_eal_tailq_unregister()` function is exported in the `.c` file with `RTE_EXPORT_SYMBOL()` but the declaration in `rte_tailq.h` lacks the `__rte_internal` tag. Functions exported with `RTE_EXPORT_SYMBOL` in the `.c` file and intended for internal use (called only by EAL macros, not application code) should be marked `__rte_internal` in the header.

**Suggested fix (in `rte_tailq.h`):**

```c
/**
 * Remove a tail queue element from the local list.
 * ...
 */
__rte_internal
void rte_eal_tailq_unregister(struct rte_tailq_elem *t);
```

---

### Warnings

#### 1. Missing release notes

This patch fixes a crash on library unload (Bugzilla 1081) and adds a new internal function. The fix should be documented in the release notes under the "Fixed Issues" section for the current release.

**Suggested addition to `doc/guides/rel_notes/release_XX_YY.rst`:**

```rst
* **Fixed crash on library unload with dynamic tailq registration.**

  Fixed a use-after-free in the EAL tailq registration system that caused
  crashes when libraries using ``EAL_REGISTER_TAILQ`` were unloaded with
  ``dlclose()`` and later reloaded.
```

#### 2. Unclear fallback logic

The comment "slot reserved by an earlier load -- reuse it" is not entirely clear. A more explicit explanation would help:

- Under what conditions does `rte_eal_tailq_create()` return `NULL` on a re-register?
- Is this expected behavior or a workaround for a limitation?

**Suggested comment improvement:**

```c
if (t->head == NULL) {
    /*
     * A previous registration of this tailq name already allocated
     * a slot in the shared mcfg->tailq_head[] array, which is keyed
     * by name and cannot be freed (shared between processes).
     * Re-attach to the existing slot instead of creating a new one.
     */
    t->head = rte_eal_tailq_lookup(t->name);
    ...
}
```

---

### Summary

**Patch 1/2:** Clean, correct fix. No issues.

**Patch 2/2:**
- **2 Errors:** Missing error check on fallback lookup; missing `__rte_internal` tag.
- **2 Warnings:** Missing release notes; unclear comment.