Fix memory leaks reported by Coverity Scan

[bryancall]

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 newed 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 (137/137) on Fedora, GCC 16.1.1.

Two of the touched plugins are not built by default, so verifying them needed extra options: uri_signing requires cjose, and jax_fingerprint defaults to off and needs -DENABLE_JAX_FINGERPRINT=ON. Both are compiled in the run above.

Draft while CI runs.

Update

Pushed a follow-up commit that replaces the explicit deletes here with a unique_ptr, since a reviewer pointed out the deletes were doing by hand what ownership should do on its own.

That turned up a leak the explicit-delete version missed. TSPluginInit's user-argument reservation failure path returned without freeing, which is only correct when the log field callback has captured the configuration, and that capture is conditional on a log symbol being configured. With no --log-field, nothing owned it and it leaked. Reserving the index before registering the log field puts every failure exit inside the span the unique_ptr still owns, so no path has to reason about who else holds a reference. Note that reordering is a behavior change, small but real: a failed reservation no longer leaves a registered log field behind.

[Copilot]

Pull request overview

This PR is the final part of a Coverity-driven cleanup series, removing a set of confirmed memory leaks across core code, plugins, and the traffic_cache_tool by ensuring caller-owned allocations are freed and by correctly owning/freeing repeatable option values.

Changes:

• Add missing cleanup for owned allocations (notably traffic_cache_tool::Cache URL set and stripe hash table).
• Fix leaks caused by repeatable command-line options overwriting previously duplicated strings in multiple plugins.
• Free caller-owned strings returned by TSMgmtStringGet and clean up plugin config on initialization failure paths.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/traffic_cache_tool/CacheTool.cc	Initializes stripes_hash_table and adds Cache destructor cleanup for URL set entries and the stripe hash table.
src/proxy/http/remap/RemapYamlConfig.cc	Avoids leaking a duplicated redirect URL buffer by passing mutable std::string storage to parse_format_redirect_url().
src/api/InkAPITest.cc	Frees the caller-owned string returned by TSMgmtStringGet() in the regression test.
plugins/xdebug/xdebug.cc	Frees previously set header name when -h/--header is specified multiple times.
plugins/remap_purge/remap_purge.cc	Frees prior option values when repeatable options overwrite instance configuration strings.
plugins/regex_revalidate/regex_revalidate.cc	Frees prior option values for repeatable options that overwrite stored configuration strings.
plugins/experimental/uri_signing/config.cc	Frees previously assigned issuer id before overwriting to prevent leaks when multiple issuers set an id.
plugins/experimental/stale_response/stale_response.cc	Frees prior log filename when the log filename option is repeated (avoids leaking prior strdup).
plugins/experimental/maxmind_acl/mmdb.cc	Frees the caller-owned TSMgmtStringGet() result in both success and fallback paths.
plugins/experimental/jax_fingerprint/plugin.cc	Deletes plugin config on additional initialization failure paths to prevent leaks.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.