RE: [PATCH] memory leak in ecpglib

Hi

On Mon. June. 10, 2019 at 09:54 AM Zhang, Jie
< zhangjie2@cn.fujitsu.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@cn.fujitsu.com]
> Sent: Monday, June 10, 2019 9:54 AM
> To: pgsql-hackers@lists.postgresql.org
> Cc: Zhang, Jie/张 杰 <zhangjie2@cn.fujitsu.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@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!
> 
>