bryancall opened a new pull request, #13552: URL: https://github.com/apache/trafficserver/pull/13552
Part 3 of 3 splitting a Coverity Scan cleanup into independently reviewable pieces. Each leaked allocation is unreachable after the leak, so releasing it changes no observable behavior. ### Repeatable command line options `regex_revalidate`, `remap_purge`, `xdebug`, `stale_response` and the `uri_signing` issuer id all overwrote a previously duplicated string when an option was given twice. Every one of these fields is null-initialized first (`memset`, an init helper, or an explicit `= nullptr`), so the first pass frees nothing and no string literal ever reaches `free`. `stale_response` compares against `PLUGIN_TAG` by pointer identity, which is the idiom its own destructor already uses. ### Ownership the callers were not honoring - `TSMgmtStringGet` hands back a copy the caller owns (`ats_strdup` internally). `maxmind_acl` and an API regression test dropped it. Both are safe on the failure path too, since the function does not write `*result` on failure and both callers pre-initialize to null. - `jax_fingerprint` leaked its configuration on three plugin initialization failure paths. Note the `reserve_user_arg` failure path is deliberately **not** given a `delete`, because the config is captured by a `TSLogFieldRegister` lambda above it. - The YAML remap parser duplicated a redirect URL that nothing owned. `parse_format_redirect_url` copies what it needs out of the buffer, so the local string's storage can be handed over directly. It does write into that buffer transiently (nul-terminating chunks in place and restoring them), so the buffer must be mutable and outlive the call, which it is and does. - `traffic_cache_tool` never released its URL set or stripe hash table. ### The one hunk where new code runs `~Cache()` in `CacheTool.cc` is the only place here that adds executing code rather than deleting or substituting. `Cache` holds `std::list<std::unique_ptr<Span>>`, so copy and move are implicitly deleted and the new destructor cannot double free; the `URLset` entries are `new`ed in exactly one place and deleted nowhere else. The `= nullptr` initializer on `stripes_hash_table` is load bearing, since most instances never build the table. Two follow-ups I did not fold in, to keep this reviewable: `build_stripe_hash_table()` overwrites `stripes_hash_table` without freeing a previous table (harmless today, one call per instance, but now that the field is owning it is worth guarding), and `regex_revalidate`'s `-l` is a fourth repeatable option that still leaks a `TSTextLogObject`. ### Verification Clean build with no new warnings and the full unit test suite passing (134/134) on Fedora, GCC 16.1.1. Every modified file is compiled, including `uri_signing`, which needs cjose present. Draft while CI runs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
