[php-src] Issue #21364: ext/snmp: minor cleanup in suffix-as-keys walk path
[email protected] (somethingwithproof)
| Newsgroups | php.bugs |
|---|---|
| Message-ID | <OyfJZEDOXMS2oE0yjbTFxWlsVa7W8Fssvf8FZzORZtM@main.internal.php.net> |
Issue: https://github.com/php/php-src/issues/21364
Author: somethingwithproof
Three pre-existing issues in the `SNMP_USE_SUFFIX_AS_KEYS | SNMP_CMD_WALK` code path in `php_snmp()` (ext/snmp/snmp.c), noticed while reviewing #21341.
**1. Redundant `snprint_objid` call**
```c
snprint_objid(buf2, sizeof(buf2), vars->name, vars->name_length); // writes full OID
if (rootlen <= vars->name_length && snmp_oid_compare(root, rootlen, vars->name, rootlen) == 0) {
// snprintf loop overwrites buf2 entirely with the suffix
```
When the `if` condition is true (the normal walk case), `buf2` is written twice and the `snprint_objid` result is discarded. The call only has effect as a fallback when the condition fails — but in that case `SUFFIX_AS_KEYS` mode silently returns a full OID as the key, mixing formats in the result array.
Suggestion: move the `snprint_objid` call into the `else` branch so it only runs when needed.
**2. Empty-suffix key collision**
When `rootlen == vars->name_length` (exact match on the walk root OID), the while loop never executes, `pos` stays 0, and `buf2[0] = '\0'`. The entry is added with key `""`. Multiple such OIDs would clobber each other in the returned array.
Worth deciding whether this case should be excluded, use the full OID as fallback, or is simply impossible in practice (in which case an assertion would document the invariant).
**3. `int count` signed/unsigned comparison**
`count` is declared `int` at line 111 but compared against `vars->name_length` (`size_t`) throughout the function, producing `-Wsign-compare` warnings. Changing to `size_t` is mechanical but touches multiple loop sites across the function.
Happy to send a PR covering all three if the approach looks reasonable.