RE: [PATCH] memory leak in ecpglib

Hi

> But I wonder if it is safe that the old cursor_name is forgotten.
old cursor_name is not assigned to other pointers, so it is safe that the old cursor_name is forgotten.

Best Regards!

-----Original Message-----
From: Matsumura, Ryo/松村 量
Sent: Monday, June 10, 2019 5:52 PM
To: Zhang, Jie/张 杰 <zhangjie2(at)cn(dot)fujitsu(dot)com>; pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Zhang, Jie/张 杰 <zhangjie2(at)cn(dot)fujitsu(dot)com>
Subject: RE: [PATCH] memory leak in ecpglib

Hi

On Mon. June. 10, 2019 at 09:54 AM Zhang, Jie < zhangjie2(at)cn(dot)fujitsu(dot)com > wrote:
>
> Memory leaks occur when the ecpg_update_declare_statement() is called
> the second time.

Certainly it is.
But I wonder if it is safe that the old cursor_name is forgotten.

Regards
Ryo Matsumura

> -----Original Message-----
> From: Zhang, Jie [mailto:zhangjie2(at)cn(dot)fujitsu(dot)com]
> Sent: Monday, June 10, 2019 9:54 AM
> To: pgsql-hackers(at)lists(dot)postgresql(dot)org
> Cc: Zhang, Jie/张 杰 <zhangjie2(at)cn(dot)fujitsu(dot)com>
> Subject: [PATCH] memory leak in ecpglib
>
> Hi all
>
> Memory leaks occur when the ecpg_update_declare_statement() is called
> the second time.
>
> FILE:postgresql\src\interfaces\ecpg\ecpglib\prepare.c
> void
> ecpg_update_declare_statement(const char *declared_name, const char
> *cursor_name, const int lineno) {
> struct declared_statement *p = NULL;
>
> if (!declared_name || !cursor_name)
> return;
>
> /* Find the declared node by declared name */
> p = ecpg_find_declared_statement(declared_name);
> if (p)
> p->cursor_name = ecpg_strdup(cursor_name, lineno); ★ }
> ecpg_strdup() returns a pointer to a null-terminated byte string,
> which is a duplicate of the string pointed to by str.
> The memory obtained is done dynamically using malloc and hence it can
> be freed using free().
>
> When the ecpg_update_declare_statement() is called for the second
> time, the memory allocated for p->cursor_name is not freed.
>
> For example:
>
> EXEC SQL BEGIN DECLARE SECTION;
> char *selectString = "SELECT * FROM foo;";
> int FooBar;
> char DooDad[17];
> EXEC SQL END DECLARE SECTION;
>
> EXEC SQL CONNECT TO postgres(at)localhost:5432 AS con1 USER postgres;
>
> EXEC SQL AT con1 DECLARE stmt_1 STATEMENT;
> EXEC SQL AT con1 PREPARE stmt_1 FROM :selectString;
>
> EXEC SQL AT con1 DECLARE cur_1 CURSOR FOR stmt_1; //★1 ECPGopen()
> --> ecpg_update_declare_statement()
> EXEC SQL AT con1 OPEN cur_1;
>
> EXEC SQL AT con1 DECLARE cur_2 CURSOR FOR stmt_1; //★2 ECPGopen()
> --> ecpg_update_declare_statement()
> EXEC SQL AT con1 OPEN cur_2;
> Memory leaks
>
> EXEC SQL FETCH cur_2 INTO:FooBar, :DooDad;
> EXEC SQL COMMIT;
> EXEC SQL DISCONNECT ALL;
>
>
> We should free p->cursor_name before p->cursor_name =
> ecpg_strdup(cursor_name, lineno).
> ######################################################################
> ###
> ####
> if(p->cursor_name)
> ecpg_free(p->cursor_name);
> p->cursor_name = ecpg_strdup(cursor_name,lineno);
> ######################################################################
> ###
> ##
> Here is a patch.
>
> Best Regards!
>
>