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

[Shruthi Gowda]

On Tue, Mar 24, 2026 at 11:29 AM Nishant Sharma <
nishant.sha...@enterprisedb.com> wrote:

> Here are some review comments on v3 patch:-
>
>    1.
>
>    Change in descriptor.c file - In my opinion, we can use `if(conn)`
>    with ecpg_raise, like other occurrence of ecpg_get_connection() call check
>    in this file, and not using ecpg_init(). Three reasons: a) Consistency in
>    checking conn after ecpg_get_connection() call in this file with if check.
>    b) We don't need to remove 'ecpg_init_sqlca(sqlca);' line due to call to
>    ecpg_init(). c) #2 comment below.
>    2.
>
>    If you agree with #1, then I see many other reasons for which
>    ECPGget_desc() returns and we can avoid ecpg_get_connection() call at top
>    of that function for those reasons and keep the check at the required
>    location only instead of moving at top of the function.
>    3.
>
>    I see there is one more location of ecpg_get_connection() call where
>    there is no check of NULL conn. In function ecpg_freeStmtCacheEntry() of
>    file prepare.c? I understand it's not required for a call in
>    ecpg_auto_prepare(), as the caller already validated that connection
>    string. But I think, conn in ecpg_freeStmtCacheEntry() is different from
>    the one that was validated.
>    4.
>
>    +1 to Mahindra, new test cases specific to the crash required for this
>    change?
>
>
>
> Regards,
> Nishant Sharma,
> EDB, Pune.
> https://www.enterprisedb.com/
>

Thanks, Nishant, for the review. I agree with points 1 and 2 and have
revised the patch accordingly. Regarding point 3, you are correct; the conn
in ecpg_freeStmtCacheEntry() differs from the one validated in the caller.
I have now added the necessary validation in the latest version.

I have also included a test case patch covering all execution paths except
for the ecpg_freeStmtCacheEntry() flow. I was unable to trigger that
specific flow, and it appears none of the existing test cases cover that
line either.

Thanks & Regards,

Shruthi K C

EnterpriseDB: http://www.enterprisedb.com

v4-0001-Add-missing-connection-validation-in-ECPG.patch
Description: Binary data

v1_test-0001-Tests-for-NULL-connection-validation.patch
Description: Binary data