[BUG] CRASH: ECPGprepared_statement() and ECPGdeallocate_all() when connection is NULL

First seen: 2025-12-08 15:38:56+00:00 · Messages: 28 · Participants: 9

Latest Update

2026-05-27 · claude-opus-4-6

New Development: Redundant ecpg_get_connection() Call in ecpg_auto_prepare()

The discussion has shifted to a code quality issue tangentially related to the original NULL-connection crash fix. Satya Narlapuram's previously-cryptic patch (noted in prior analysis #3) apparently proposed adding a NULL-connection validation check inside ecpg_auto_prepare(). This has now generated substantive technical discussion.

Shruthi's Response to Satya

Shruthi rejects the patch as redundant: the caller ecpg_do_prologue() already validates the connection before calling ecpg_auto_prepare(), so adding another check inside would be defensive-in-depth at the cost of dead code.

Michael Paquier's Architectural Observation

Michael Paquier (a committer) raises a deeper point: the real problem isn't whether to add a NULL check in ecpg_auto_prepare() — it's that ecpg_auto_prepare() calls ecpg_get_connection() a second time to re-lookup a connection that ecpg_do_prologue() already resolved. This is wasteful (it's a cache/list lookup) and architecturally wrong — the connection should be passed as a resolved pointer, not re-resolved by name.

Michael proposes refactoring ecpg_auto_prepare() to accept the connection object directly instead of the connection name, eliminating the redundant lookup entirely.

Shruthi's Rebuttal: Signature Constraints

Shruthi agrees in principle but identifies a practical obstacle: functions called inside ecpg_auto_prepare() — specifically ECPGprepare() and AddStmtToCache() — expect a connection name (string), not a connection pointer. ECPGprepare() is a public API corresponding to EXEC SQL PREPARE, so it must perform its own ecpg_get_connection() lookup internally. Therefore, even if the connection pointer is passed in, the name is still needed, and the inner ECPGprepare() will redundantly re-lookup anyway.

This highlights a broader design tension in libecpg: public entry points (which must accept connection names and do their own lookups) are also called internally from code paths that have already resolved the connection. There's no internal-only variant that accepts a pre-resolved struct connection *. Fixing the redundancy fully would require introducing internal _conn variants of public functions — a larger refactoring.

Status

No patch has been posted for this refactoring. The discussion is exploratory. Michael's request for a regression test demonstrating any actual failure if the extra ecpg_init() were needed suggests he wants proof the current code is safe before accepting the status quo.

History (1 prior analysis)
2026-05-25 · claude-opus-4-6

Minimal New Activity: Brief Post from Satya Narlapuram

A new participant (Satya Narlapuram) has posted a very brief message stating "Looks like this committed a case, attached a patch to fix this." The message is cryptic and lacks technical detail — it does not specify which "case" was committed, what the patch fixes, or provide any description of the change. Without the actual patch content being visible in the message text, no substantive technical analysis is possible beyond noting the contribution.

The phrasing "committed a case" likely refers to a typo/oversight introduced by the original commit (Andrew Dunstan's 2026-05-01 push), and the patch presumably addresses a minor post-commit issue. However, without patch contents or further context, this is speculative.

No other responses, reviews, or discussion have followed in this round.