Re: ECPG cleanup and fix for clang compile-time problem - Mailing list pgsql-hackers

From Tom Lane
Subject Re: ECPG cleanup and fix for clang compile-time problem
Date
Msg-id 200977.1729101261@sss.pgh.pa.us
Whole thread Raw
In response to Re: ECPG cleanup and fix for clang compile-time problem  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
I wrote:
> Attached are rebased and renumbered 0006-0008, mostly to keep the
> cfbot happy.

For some reason I thought the stuff I pushed later on Monday
didn't interact with these patches, but the cfbot disabused me
of that folly.  Here's a rebased v7 --- no substantive change.

            regards, tom lane

From 52e50770ca723278cf04da52fee0ffaddfae8235 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 16 Oct 2024 13:38:13 -0400
Subject: [PATCH v7 1/3] ecpg: fix some memory leakage of data-type-related
 structures.

ECPGfree_type() and related functions were quite incomplete
about removing subsidiary data structures.  Possibly this is
because ecpg wasn't careful to make sure said data structures
always had their own storage.  Previous patches in this series
cleaned up a lot of that, and I had to add a couple more
mm_strdup's here.

Also, ecpg.trailer tended to overwrite struct_member_list[struct_level]
without bothering to free up its previous contents, thus potentially
leaking a lot of struct-member-related storage.  Add
ECPGfree_struct_member() calls at appropriate points.  (Note: the
lifetime of those lists is not obvious.  They are still live after
initial construction, in order to handle cases like

struct foo { ... } foovar1, foovar2;

We can't delete the list immediately after parsing the right brace,
because it needs to be copied for each of the variables.  Instead,
it's kept around until the next struct declaration.)

Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
---
 src/interfaces/ecpg/preproc/ecpg.header  |  3 ++-
 src/interfaces/ecpg/preproc/ecpg.trailer | 11 +++++++++--
 src/interfaces/ecpg/preproc/type.c       | 15 +++++++++------
 src/interfaces/ecpg/preproc/type.h       |  5 +++--
 src/interfaces/ecpg/preproc/variable.c   |  7 ++++++-
 5 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/src/interfaces/ecpg/preproc/ecpg.header b/src/interfaces/ecpg/preproc/ecpg.header
index a415780faf..9554e2b02e 100644
--- a/src/interfaces/ecpg/preproc/ecpg.header
+++ b/src/interfaces/ecpg/preproc/ecpg.header
@@ -507,11 +507,12 @@ add_typedef(const char *name, const char *dimension, const char *length,
         this->name = mm_strdup(name);
         this->brace_level = braces_open;
         this->type = (struct this_type *) mm_alloc(sizeof(struct this_type));
+        this->type->type_storage = NULL;
         this->type->type_enum = type_enum;
         this->type->type_str = mm_strdup(name);
         this->type->type_dimension = mm_strdup(dimension); /* dimension of array */
         this->type->type_index = mm_strdup(length);    /* length of string */
-        this->type->type_sizeof = ECPGstruct_sizeof;
+        this->type->type_sizeof = ECPGstruct_sizeof ? mm_strdup(ECPGstruct_sizeof) : NULL;
         this->struct_member_list = (type_enum == ECPGt_struct || type_enum == ECPGt_union) ?
             ECPGstruct_member_dup(struct_member_list[struct_level]) : NULL;

diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
index e466668ea2..0b15252433 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -752,6 +752,7 @@ var_type: simple_type
             else
                 $$.type_sizeof = cat_str(3, "sizeof(", this->name, ")");

+            ECPGfree_struct_member(struct_member_list[struct_level]);
             struct_member_list[struct_level] = ECPGstruct_member_dup(this->struct_member_list);
         }
     }
@@ -879,6 +880,7 @@ var_type: simple_type
             else
                 $$.type_sizeof = cat_str(3, "sizeof(", this->name, ")");

+            ECPGfree_struct_member(struct_member_list[struct_level]);
             struct_member_list[struct_level] = ECPGstruct_member_dup(this->struct_member_list);
         }
     }
@@ -901,6 +903,7 @@ var_type: simple_type
             $$.type_dimension = this->type->type_dimension;
             $$.type_index = this->type->type_index;
             $$.type_sizeof = this->type->type_sizeof;
+            ECPGfree_struct_member(struct_member_list[struct_level]);
             struct_member_list[struct_level] = ECPGstruct_member_dup(this->struct_member_list);
         }
         else
@@ -910,6 +913,7 @@ var_type: simple_type
             $$.type_dimension = "-1";
             $$.type_index = "-1";
             $$.type_sizeof = "";
+            ECPGfree_struct_member(struct_member_list[struct_level]);
             struct_member_list[struct_level] = NULL;
         }
     }
@@ -925,6 +929,7 @@ enum_definition: '{' c_list '}'

 struct_union_type_with_symbol: s_struct_union_symbol
     {
+        ECPGfree_struct_member(struct_member_list[struct_level]);
         struct_member_list[struct_level++] = NULL;
         if (struct_level >= STRUCT_DEPTH)
             mmerror(PARSE_ERROR, ET_ERROR, "too many levels in nested structure/union definition");
@@ -966,12 +971,13 @@ struct_union_type_with_symbol: s_struct_union_symbol
         this->name = mm_strdup(su_type.type_str);
         this->brace_level = braces_open;
         this->type = (struct this_type *) mm_alloc(sizeof(struct this_type));
+        this->type->type_storage = NULL;
         this->type->type_enum = su_type.type_enum;
         this->type->type_str = mm_strdup(su_type.type_str);
         this->type->type_dimension = mm_strdup("-1");    /* dimension of array */
         this->type->type_index = mm_strdup("-1");    /* length of string */
-        this->type->type_sizeof = ECPGstruct_sizeof;
-        this->struct_member_list = struct_member_list[struct_level];
+        this->type->type_sizeof = ECPGstruct_sizeof ? mm_strdup(ECPGstruct_sizeof) : NULL;
+        this->struct_member_list = ECPGstruct_member_dup(struct_member_list[struct_level]);

         types = this;
         @$ = cat_str(4, su_type.type_str, "{", @4, "}");
@@ -981,6 +987,7 @@ struct_union_type_with_symbol: s_struct_union_symbol
 struct_union_type: struct_union_type_with_symbol
     | s_struct_union
     {
+        ECPGfree_struct_member(struct_member_list[struct_level]);
         struct_member_list[struct_level++] = NULL;
         if (struct_level >= STRUCT_DEPTH)
             mmerror(PARSE_ERROR, ET_ERROR, "too many levels in nested structure/union definition");
diff --git a/src/interfaces/ecpg/preproc/type.c b/src/interfaces/ecpg/preproc/type.c
index 7f52521dbf..9f6dacd2ae 100644
--- a/src/interfaces/ecpg/preproc/type.c
+++ b/src/interfaces/ecpg/preproc/type.c
@@ -94,13 +94,14 @@ ECPGmake_array_type(struct ECPGtype *type, const char *size)
 }

 struct ECPGtype *
-ECPGmake_struct_type(struct ECPGstruct_member *rm, enum ECPGttype type, char *type_name, char *struct_sizeof)
+ECPGmake_struct_type(struct ECPGstruct_member *rm, enum ECPGttype type,
+                     const char *type_name, const char *struct_sizeof)
 {
     struct ECPGtype *ne = ECPGmake_simple_type(type, "1", 0);

     ne->type_name = mm_strdup(type_name);
     ne->u.members = ECPGstruct_member_dup(rm);
-    ne->struct_sizeof = struct_sizeof;
+    ne->struct_sizeof = mm_strdup(struct_sizeof);

     return ne;
 }
@@ -622,7 +623,7 @@ ECPGfree_struct_member(struct ECPGstruct_member *rm)

         rm = rm->next;
         free(p->name);
-        free(p->type);
+        ECPGfree_type(p->type);
         free(p);
     }
 }
@@ -643,14 +644,13 @@ ECPGfree_type(struct ECPGtype *type)
                     case ECPGt_struct:
                     case ECPGt_union:
                         /* Array of structs. */
-                        ECPGfree_struct_member(type->u.element->u.members);
-                        free(type->u.element);
+                        ECPGfree_type(type->u.element);
                         break;
                     default:
                         if (!IS_SIMPLE_TYPE(type->u.element->type))
                             base_yyerror("internal error: unknown datatype, please report this to <" PACKAGE_BUGREPORT
">");

-                        free(type->u.element);
+                        ECPGfree_type(type->u.element);
                 }
                 break;
             case ECPGt_struct:
@@ -662,6 +662,9 @@ ECPGfree_type(struct ECPGtype *type)
                 break;
         }
     }
+    free(type->type_name);
+    free(type->size);
+    free(type->struct_sizeof);
     free(type);
 }

diff --git a/src/interfaces/ecpg/preproc/type.h b/src/interfaces/ecpg/preproc/type.h
index 90126551d1..3d99e1703d 100644
--- a/src/interfaces/ecpg/preproc/type.h
+++ b/src/interfaces/ecpg/preproc/type.h
@@ -38,8 +38,9 @@ void        ECPGmake_struct_member(const char *name, struct ECPGtype *type,
 struct ECPGtype *ECPGmake_simple_type(enum ECPGttype type, const char *size, int counter);
 struct ECPGtype *ECPGmake_array_type(struct ECPGtype *type, const char *size);
 struct ECPGtype *ECPGmake_struct_type(struct ECPGstruct_member *rm,
-                                      enum ECPGttype type, char *type_name,
-                                      char *struct_sizeof);
+                                      enum ECPGttype type,
+                                      const char *type_name,
+                                      const char *struct_sizeof);
 struct ECPGstruct_member *ECPGstruct_member_dup(struct ECPGstruct_member *rm);

 /* Frees a type. */
diff --git a/src/interfaces/ecpg/preproc/variable.c b/src/interfaces/ecpg/preproc/variable.c
index a4294b8f0f..2c129646d2 100644
--- a/src/interfaces/ecpg/preproc/variable.c
+++ b/src/interfaces/ecpg/preproc/variable.c
@@ -276,7 +276,12 @@ remove_typedefs(int brace_level)
                 prev->next = p->next;

             if (p->type->type_enum == ECPGt_struct || p->type->type_enum == ECPGt_union)
-                free(p->struct_member_list);
+                ECPGfree_struct_member(p->struct_member_list);
+            free(p->type->type_storage);
+            free(p->type->type_str);
+            free(p->type->type_dimension);
+            free(p->type->type_index);
+            free(p->type->type_sizeof);
             free(p->type);
             free(p->name);
             free(p);
--
2.43.5

From 1a2904611f307ec2b0dc31f04094d90e0ce178ea Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 16 Oct 2024 13:39:55 -0400
Subject: [PATCH v7 2/3] ecpg: put all string-valued tokens returned by pgc.l
 in local storage.

This didn't work earlier in the patch series (I think some of
the strings were ending up in data-type-related structures),
but apparently we're now clean enough for it.  This considerably
reduces process-lifespan memory leakage.

Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
---
 src/interfaces/ecpg/preproc/parser.c |  4 ++-
 src/interfaces/ecpg/preproc/pgc.l    | 42 ++++++++++++++--------------
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/src/interfaces/ecpg/preproc/parser.c b/src/interfaces/ecpg/preproc/parser.c
index 373c93fc04..181417fb39 100644
--- a/src/interfaces/ecpg/preproc/parser.c
+++ b/src/interfaces/ecpg/preproc/parser.c
@@ -203,7 +203,9 @@ filtered_base_yylex(void)
                 base_yytext = cur_yytext;

                 /* Combine 3 tokens into 1 */
-                base_yylval.str = psprintf("%s UESCAPE %s", base_yylval.str, escstr);
+                base_yylval.str = make3_str(base_yylval.str,
+                                            " UESCAPE ",
+                                            escstr);
                 base_yylloc = loc_strdup(base_yylval.str);

                 /* Clear have_lookahead, thereby consuming all three tokens */
diff --git a/src/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index f3c03482ae..82708013ee 100644
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -641,26 +641,26 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                         case xb:
                             if (literalbuf[strspn(literalbuf, "01")] != '\0')
                                 mmerror(PARSE_ERROR, ET_ERROR, "invalid bit string literal");
-                            base_yylval.str = psprintf("b'%s'", literalbuf);
+                            base_yylval.str = make3_str("b'", literalbuf, "'");
                             return BCONST;
                         case xh:
                             if (literalbuf[strspn(literalbuf, "0123456789abcdefABCDEF")] != '\0')
                                 mmerror(PARSE_ERROR, ET_ERROR, "invalid hexadecimal string literal");
-                            base_yylval.str = psprintf("x'%s'", literalbuf);
+                            base_yylval.str = make3_str("x'", literalbuf, "'");
                             return XCONST;
                         case xq:
                             /* fallthrough */
                         case xqc:
-                            base_yylval.str = psprintf("'%s'", literalbuf);
+                            base_yylval.str = make3_str("'", literalbuf, "'");
                             return SCONST;
                         case xe:
-                            base_yylval.str = psprintf("E'%s'", literalbuf);
+                            base_yylval.str = make3_str("E'", literalbuf, "'");
                             return SCONST;
                         case xn:
-                            base_yylval.str = psprintf("N'%s'", literalbuf);
+                            base_yylval.str = make3_str("N'", literalbuf, "'");
                             return SCONST;
                         case xus:
-                            base_yylval.str = psprintf("U&'%s'", literalbuf);
+                            base_yylval.str = make3_str("U&'", literalbuf, "'");
                             return USCONST;
                         default:
                             mmfatal(PARSE_ERROR, "unhandled previous state in xqs\n");
@@ -724,7 +724,7 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                         free(dolqstart);
                         dolqstart = NULL;
                         BEGIN(SQL);
-                        base_yylval.str = mm_strdup(literalbuf);
+                        base_yylval.str = loc_strdup(literalbuf);
                         return SCONST;
                     }
                     else
@@ -778,12 +778,12 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                      * PREPARE and EXECUTE IMMEDIATE, which can certainly be
                      * longer than NAMEDATALEN.
                      */
-                    base_yylval.str = mm_strdup(literalbuf);
+                    base_yylval.str = loc_strdup(literalbuf);
                     return CSTRING;
                 }
 <xdc>{xdstop}    {
                     BEGIN(state_before_str_start);
-                    base_yylval.str = mm_strdup(literalbuf);
+                    base_yylval.str = loc_strdup(literalbuf);
                     return CSTRING;
                 }
 <xui>{dquote}    {
@@ -795,7 +795,7 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                      * The backend will truncate the identifier here. We do
                      * not as it does not change the result.
                      */
-                    base_yylval.str = psprintf("U&\"%s\"", literalbuf);
+                    base_yylval.str = make3_str("U&\"", literalbuf, "\"");
                     return UIDENT;
                 }
 <xd,xui>{xddouble} {
@@ -971,7 +971,7 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                         }
                     }

-                    base_yylval.str = mm_strdup(yytext);
+                    base_yylval.str = loc_strdup(yytext);
                     return Op;
                 }

@@ -990,7 +990,7 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                 }

 {ip}            {
-                    base_yylval.str = mm_strdup(yytext);
+                    base_yylval.str = loc_strdup(yytext);
                     return IP;
                 }
 }  /* <SQL> */
@@ -1003,7 +1003,7 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                     return process_integer_literal(yytext, &base_yylval, 16);
                 }
 {numeric}        {
-                    base_yylval.str = mm_strdup(yytext);
+                    base_yylval.str = loc_strdup(yytext);
                     return FCONST;
                 }
 {numericfail}    {
@@ -1012,7 +1012,7 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                     return process_integer_literal(yytext, &base_yylval, 10);
                 }
 {real}            {
-                    base_yylval.str = mm_strdup(yytext);
+                    base_yylval.str = loc_strdup(yytext);
                     return FCONST;
                 }
 {realfail}        {
@@ -1048,7 +1048,7 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                 }

 :{identifier}((("->"|\.){identifier})|(\[{array}\]))*    {
-                    base_yylval.str = mm_strdup(yytext + 1);
+                    base_yylval.str = loc_strdup(yytext + 1);
                     return CVARIABLE;
                 }

@@ -1085,7 +1085,7 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                          * to do so; that's just another way that ecpg could
                          * get out of step with the backend.
                          */
-                        base_yylval.str = mm_strdup(yytext);
+                        base_yylval.str = loc_strdup(yytext);
                         return IDENT;
                     }
                 }
@@ -1124,7 +1124,7 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                     }
                     else
                     {
-                        base_yylval.str = mm_strdup(yytext);
+                        base_yylval.str = loc_strdup(yytext);
                         return CPP_LINE;
                     }
                 }
@@ -1136,12 +1136,12 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                     }
                     else
                     {
-                        base_yylval.str = mm_strdup(yytext);
+                        base_yylval.str = loc_strdup(yytext);
                         return CPP_LINE;
                     }
                 }
 <C,SQL>{cppline} {
-                    base_yylval.str = mm_strdup(yytext);
+                    base_yylval.str = loc_strdup(yytext);
                     return CPP_LINE;
                 }
 <C>{identifier}    {
@@ -1167,7 +1167,7 @@ cppline            {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
                             return kwvalue;
                         else
                         {
-                            base_yylval.str = mm_strdup(yytext);
+                            base_yylval.str = loc_strdup(yytext);
                             return IDENT;
                         }
                     }
@@ -1685,7 +1685,7 @@ process_integer_literal(const char *token, YYSTYPE *lval, int base)
     if (*endptr != '\0' || errno == ERANGE)
     {
         /* integer too large (or contains decimal pt), treat it as a float */
-        lval->str = mm_strdup(token);
+        lval->str = loc_strdup(token);
         return FCONST;
     }
     lval->ival = val;
--
2.43.5

From f4454425adbcf57a3537cd3a00c131e8754f8154 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 16 Oct 2024 13:48:33 -0400
Subject: [PATCH v7 3/3] ecpg: clean up some other assorted memory leaks.

Avoid leaking the prior value when updating the "connection"
state variable.

Ditto for ECPGstruct_sizeof.  (It seems like this one ought to
be statement-local, but testing says it isn't, and I didn't
feel like diving deeper.)

The actual_type[] entries are statement-local, though, so
no need to mm_strdup() strings stored in them.

Likewise, sqlda variables are statement-local, so we can
loc_alloc them.

Also clean up sloppiness around management of the argsinsert and
argsresult lists.

progname changes are strictly to prevent valgrind from complaining
about leaked allocations.

With this, valgrind reports zero leakage in the ecpg preprocessor
for all of our ecpg regression test cases.

Discussion: https://postgr.es/m/2011420.1713493114@sss.pgh.pa.us
---
 src/interfaces/ecpg/preproc/descriptor.c | 14 +++++--
 src/interfaces/ecpg/preproc/ecpg.addons  | 47 +++++++++---------------
 src/interfaces/ecpg/preproc/ecpg.c       |  2 +-
 src/interfaces/ecpg/preproc/ecpg.header  | 17 ++++++++-
 src/interfaces/ecpg/preproc/ecpg.trailer | 31 +++++++++-------
 src/interfaces/ecpg/preproc/output.c     |  3 +-
 src/interfaces/ecpg/preproc/variable.c   | 25 +++++++++++--
 7 files changed, 86 insertions(+), 53 deletions(-)

diff --git a/src/interfaces/ecpg/preproc/descriptor.c b/src/interfaces/ecpg/preproc/descriptor.c
index 9b87d07d09..e8c7016bdc 100644
--- a/src/interfaces/ecpg/preproc/descriptor.c
+++ b/src/interfaces/ecpg/preproc/descriptor.c
@@ -344,11 +344,17 @@ descriptor_variable(const char *name, int input)
 struct variable *
 sqlda_variable(const char *name)
 {
-    struct variable *p = (struct variable *) mm_alloc(sizeof(struct variable));
-
-    p->name = mm_strdup(name);
-    p->type = (struct ECPGtype *) mm_alloc(sizeof(struct ECPGtype));
+    /*
+     * Presently, sqlda variables are only needed for the duration of the
+     * current statement.  Rather than add infrastructure to manage them,
+     * let's just loc_alloc them.
+     */
+    struct variable *p = (struct variable *) loc_alloc(sizeof(struct variable));
+
+    p->name = loc_strdup(name);
+    p->type = (struct ECPGtype *) loc_alloc(sizeof(struct ECPGtype));
     p->type->type = ECPGt_sqlda;
+    p->type->type_name = NULL;
     p->type->size = NULL;
     p->type->struct_sizeof = NULL;
     p->type->u.element = NULL;
diff --git a/src/interfaces/ecpg/preproc/ecpg.addons b/src/interfaces/ecpg/preproc/ecpg.addons
index 935820c8d4..71f7ad26ad 100644
--- a/src/interfaces/ecpg/preproc/ecpg.addons
+++ b/src/interfaces/ecpg/preproc/ecpg.addons
@@ -135,6 +135,7 @@ ECPG: rule stmt ViewStmt

         fprintf(base_yyout, "{ ECPGdescribe(__LINE__, %d, %d, %s, %s,", compat, $1.input, connection ? connection :
"NULL",$1.stmt_name); 
         dump_variables(argsresult, 1);
+        argsresult = NULL;
         fputs("ECPGt_EORT);", base_yyout);
         fprintf(base_yyout, "}");
         output_line_number();
@@ -181,6 +182,7 @@ ECPG: rule stmt ViewStmt

         if ((ptr = add_additional_variables(@1, true)) != NULL)
         {
+            free(connection);
             connection = ptr->connection ? mm_strdup(ptr->connection) : NULL;
             output_statement(ptr->command, 0, ECPGst_normal);
             ptr->opened = true;
@@ -247,15 +249,13 @@ ECPG: addon var_value NumericOnly
 ECPG: addon fetch_args cursor_name
         struct cursor *ptr = add_additional_variables(@1, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);
         if (@1[0] == ':')
             @$ = "$0";
 ECPG: addon fetch_args from_in cursor_name
         struct cursor *ptr = add_additional_variables(@2, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);
         if (@2[0] == ':')
             @$ = cat2_str(@1, "$0");
 ECPG: addon fetch_args NEXT opt_from_in cursor_name
@@ -265,16 +265,14 @@ ECPG: addon fetch_args LAST_P opt_from_in cursor_name
 ECPG: addon fetch_args ALL opt_from_in cursor_name
         struct cursor *ptr = add_additional_variables(@3, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);
         if (@3[0] == ':')
             @$ = cat_str(3, @1, @2, "$0");
 ECPG: addon fetch_args SignedIconst opt_from_in cursor_name
         struct cursor *ptr = add_additional_variables(@3, false);
         bool    replace = false;

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);
         if (@3[0] == ':')
         {
             @3 = "$0";
@@ -291,8 +289,7 @@ ECPG: addon fetch_args FORWARD ALL opt_from_in cursor_name
 ECPG: addon fetch_args BACKWARD ALL opt_from_in cursor_name
         struct cursor *ptr = add_additional_variables(@4, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);
         if (@4[0] == ':')
             @$ = cat_str(4, @1, @2, @3, "$0");
 ECPG: addon fetch_args ABSOLUTE_P SignedIconst opt_from_in cursor_name
@@ -302,8 +299,7 @@ ECPG: addon fetch_args BACKWARD SignedIconst opt_from_in cursor_name
         struct cursor *ptr = add_additional_variables(@4, false);
         bool    replace = false;

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);
         if (@4[0] == ':')
         {
             @4 = "$0";
@@ -412,8 +408,7 @@ ECPG: block ClosePortalStmt CLOSE cursor_name
         {
             if (strcmp(@2, ptr->name) == 0)
             {
-                if (ptr->connection)
-                    connection = mm_strdup(ptr->connection);
+                update_connection(ptr->connection);
                 break;
             }
         }
@@ -483,8 +478,7 @@ ECPG: rule FetchStmt MOVE fetch_args
         const char *cursor_marker = @3[0] == ':' ? "$0" : @3;
         struct cursor *ptr = add_additional_variables(@3, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);

         @$ = cat_str(2, "fetch forward", cursor_marker);
     }
@@ -493,8 +487,7 @@ ECPG: rule FetchStmt MOVE fetch_args
         const char *cursor_marker = @4[0] == ':' ? "$0" : @4;
         struct cursor *ptr = add_additional_variables(@4, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);

         @$ = cat_str(2, "fetch forward from", cursor_marker);
     }
@@ -503,8 +496,7 @@ ECPG: rule FetchStmt MOVE fetch_args
         const char *cursor_marker = @3[0] == ':' ? "$0" : @3;
         struct cursor *ptr = add_additional_variables(@3, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);

         @$ = cat_str(2, "fetch backward", cursor_marker);
     }
@@ -513,8 +505,7 @@ ECPG: rule FetchStmt MOVE fetch_args
         const char *cursor_marker = @4[0] == ':' ? "$0" : @4;
         struct cursor *ptr = add_additional_variables(@4, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);

         @$ = cat_str(2, "fetch backward from", cursor_marker);
     }
@@ -523,8 +514,7 @@ ECPG: rule FetchStmt MOVE fetch_args
         const char *cursor_marker = @3[0] == ':' ? "$0" : @3;
         struct cursor *ptr = add_additional_variables(@3, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);

         @$ = cat_str(2, "move forward", cursor_marker);
     }
@@ -533,8 +523,7 @@ ECPG: rule FetchStmt MOVE fetch_args
         const char *cursor_marker = @4[0] == ':' ? "$0" : @4;
         struct cursor *ptr = add_additional_variables(@4, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);

         @$ = cat_str(2, "move forward from", cursor_marker);
     }
@@ -543,8 +532,7 @@ ECPG: rule FetchStmt MOVE fetch_args
         const char *cursor_marker = @3[0] == ':' ? "$0" : @3;
         struct cursor *ptr = add_additional_variables(@3, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);

         @$ = cat_str(2, "move backward", cursor_marker);
     }
@@ -553,8 +541,7 @@ ECPG: rule FetchStmt MOVE fetch_args
         const char *cursor_marker = @4[0] == ':' ? "$0" : @4;
         struct cursor *ptr = add_additional_variables(@4, false);

-        if (ptr->connection)
-            connection = mm_strdup(ptr->connection);
+        update_connection(ptr->connection);

         @$ = cat_str(2, "move backward from", cursor_marker);
     }
diff --git a/src/interfaces/ecpg/preproc/ecpg.c b/src/interfaces/ecpg/preproc/ecpg.c
index 73c37631ac..2fcc6f8f99 100644
--- a/src/interfaces/ecpg/preproc/ecpg.c
+++ b/src/interfaces/ecpg/preproc/ecpg.c
@@ -20,6 +20,7 @@ bool        autocommit = false,
             regression_mode = false,
             auto_prepare = false;

+static const char *progname;
 char       *output_filename;

 enum COMPAT_MODE compat = ECPG_COMPAT_PGSQL;
@@ -139,7 +140,6 @@ main(int argc, char *const argv[])
     bool        verbose = false,
                 header_mode = false;
     struct _include_path *ip;
-    const char *progname;
     char        my_exec_path[MAXPGPATH];
     char        include_path[MAXPGPATH];

diff --git a/src/interfaces/ecpg/preproc/ecpg.header b/src/interfaces/ecpg/preproc/ecpg.header
index 9554e2b02e..e002e0d835 100644
--- a/src/interfaces/ecpg/preproc/ecpg.header
+++ b/src/interfaces/ecpg/preproc/ecpg.header
@@ -59,6 +59,7 @@ struct variable no_indicator = {"no_indicator", &ecpg_no_indicator, 0, NULL};
 static struct ECPGtype ecpg_query = {ECPGt_char_variable, NULL, NULL, NULL, {NULL}, 0};

 static bool check_declared_list(const char *name);
+static void update_connection(const char *newconn);


 /*
@@ -546,12 +547,26 @@ check_declared_list(const char *name)
         {
             if (connection && strcmp(ptr->connection, connection) != 0)
                 mmerror(PARSE_ERROR, ET_WARNING, "connection %s is overwritten with %s by DECLARE statement %s",
connection,ptr->connection, name); 
-            connection = mm_strdup(ptr->connection);
+            update_connection(ptr->connection);
             return true;
         }
     }
     return false;
 }
+
+/*
+ * If newconn isn't NULL, update the global "connection" variable to that;
+ * otherwise do nothing.
+ */
+static void
+update_connection(const char *newconn)
+{
+    if (newconn)
+    {
+        free(connection);
+        connection = mm_strdup(newconn);
+    }
+}
 %}

 %expect 0
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer
index 0b15252433..95630c1e66 100644
--- a/src/interfaces/ecpg/preproc/ecpg.trailer
+++ b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -74,6 +74,8 @@ CreateAsStmt: CREATE OptTemp TABLE create_as_target AS

 at: AT connection_object
     {
+        if (connection)
+            free(connection);
         connection = mm_strdup(@2);

         /*
@@ -557,13 +559,12 @@ type_declaration: S_TYPEDEF
 var_declaration:
     storage_declaration var_type
     {
-        actual_type[struct_level].type_storage = mm_strdup(@1);
+        actual_type[struct_level].type_storage = loc_strdup(@1);
         actual_type[struct_level].type_enum = $2.type_enum;
-        actual_type[struct_level].type_str = mm_strdup($2.type_str);
-        actual_type[struct_level].type_dimension = mm_strdup($2.type_dimension);
-        actual_type[struct_level].type_index = mm_strdup($2.type_index);
-        actual_type[struct_level].type_sizeof =
-            $2.type_sizeof ? mm_strdup($2.type_sizeof) : NULL;
+        actual_type[struct_level].type_str = $2.type_str;
+        actual_type[struct_level].type_dimension = $2.type_dimension;
+        actual_type[struct_level].type_index = $2.type_index;
+        actual_type[struct_level].type_sizeof = $2.type_sizeof;

         actual_startline[struct_level] = hashline_number();
     }
@@ -573,13 +574,12 @@ var_declaration:
     }
     | var_type
     {
-        actual_type[struct_level].type_storage = mm_strdup("");
+        actual_type[struct_level].type_storage = loc_strdup("");
         actual_type[struct_level].type_enum = $1.type_enum;
-        actual_type[struct_level].type_str = mm_strdup($1.type_str);
-        actual_type[struct_level].type_dimension = mm_strdup($1.type_dimension);
-        actual_type[struct_level].type_index = mm_strdup($1.type_index);
-        actual_type[struct_level].type_sizeof =
-            $1.type_sizeof ? mm_strdup($1.type_sizeof) : NULL;
+        actual_type[struct_level].type_str = $1.type_str;
+        actual_type[struct_level].type_dimension = $1.type_dimension;
+        actual_type[struct_level].type_index = $1.type_index;
+        actual_type[struct_level].type_sizeof = $1.type_sizeof;

         actual_startline[struct_level] = hashline_number();
     }
@@ -871,7 +871,7 @@ var_type: simple_type
             /* Otherwise, it must be a user-defined typedef name */
             struct typedefs *this = get_typedef(@1, false);

-            $$.type_str = (this->type->type_enum == ECPGt_varchar || this->type->type_enum == ECPGt_bytea) ?
mm_strdup(""): mm_strdup(this->name); 
+            $$.type_str = (this->type->type_enum == ECPGt_varchar || this->type->type_enum == ECPGt_bytea) ? "" :
this->name;
             $$.type_enum = this->type->type_enum;
             $$.type_dimension = this->type->type_dimension;
             $$.type_index = this->type->type_index;
@@ -1005,6 +1005,7 @@ s_struct_union_symbol: SQL_STRUCT symbol
     {
         $$.su = "struct";
         $$.symbol = @2;
+        free(ECPGstruct_sizeof);
         ECPGstruct_sizeof = mm_strdup(cat_str(3, "sizeof(",
                                               cat2_str($$.su, $$.symbol),
                                               ")"));
@@ -1018,6 +1019,7 @@ s_struct_union_symbol: SQL_STRUCT symbol

 s_struct_union: SQL_STRUCT
     {
+        free(ECPGstruct_sizeof);
         ECPGstruct_sizeof = mm_strdup("");    /* This must not be NULL to
                                              * distinguish from simple types. */
         @$ = "struct";
@@ -1697,18 +1699,21 @@ ECPGVar: SQL_VAR
 ECPGWhenever: SQL_WHENEVER SQL_SQLERROR action
     {
         when_error.code = $3.code;
+        free(when_error.command);
         when_error.command = $3.command ? mm_strdup($3.command) : NULL;
         @$ = cat_str(3, "/* exec sql whenever sqlerror ", $3.str, "; */");
     }
     | SQL_WHENEVER NOT SQL_FOUND action
     {
         when_nf.code = $4.code;
+        free(when_nf.command);
         when_nf.command = $4.command ? mm_strdup($4.command) : NULL;
         @$ = cat_str(3, "/* exec sql whenever not found ", $4.str, "; */");
     }
     | SQL_WHENEVER SQL_SQLWARNING action
     {
         when_warn.code = $3.code;
+        free(when_warn.command);
         when_warn.command = $3.command ? mm_strdup($3.command) : NULL;
         @$ = cat_str(3, "/* exec sql whenever sql_warning ", $3.str, "; */");
     }
diff --git a/src/interfaces/ecpg/preproc/output.c b/src/interfaces/ecpg/preproc/output.c
index a18904f88b..b190e9f0ce 100644
--- a/src/interfaces/ecpg/preproc/output.c
+++ b/src/interfaces/ecpg/preproc/output.c
@@ -155,10 +155,11 @@ output_statement(const char *stmt, int whenever_mode, enum ECPG_statement_type s

     /* dump variables to C file */
     dump_variables(argsinsert, 1);
+    argsinsert = NULL;
     fputs("ECPGt_EOIT, ", base_yyout);
     dump_variables(argsresult, 1);
+    argsresult = NULL;
     fputs("ECPGt_EORT);", base_yyout);
-    reset_variables();

     whenever_action(whenever_mode | 2);
 }
diff --git a/src/interfaces/ecpg/preproc/variable.c b/src/interfaces/ecpg/preproc/variable.c
index 2c129646d2..3bed096d95 100644
--- a/src/interfaces/ecpg/preproc/variable.c
+++ b/src/interfaces/ecpg/preproc/variable.c
@@ -314,10 +314,12 @@ remove_variables(int brace_level)
             for (ptr = cur; ptr != NULL; ptr = ptr->next)
             {
                 struct arguments *varptr,
-                           *prevvar;
+                           *prevvar,
+                           *nextvar;

-                for (varptr = prevvar = ptr->argsinsert; varptr != NULL; varptr = varptr->next)
+                for (varptr = prevvar = ptr->argsinsert; varptr != NULL; varptr = nextvar)
                 {
+                    nextvar = varptr->next;
                     if (p == varptr->variable)
                     {
                         /* remove from list */
@@ -325,10 +327,12 @@ remove_variables(int brace_level)
                             ptr->argsinsert = varptr->next;
                         else
                             prevvar->next = varptr->next;
+                        free(varptr);
                     }
                 }
-                for (varptr = prevvar = ptr->argsresult; varptr != NULL; varptr = varptr->next)
+                for (varptr = prevvar = ptr->argsresult; varptr != NULL; varptr = nextvar)
                 {
+                    nextvar = varptr->next;
                     if (p == varptr->variable)
                     {
                         /* remove from list */
@@ -336,6 +340,7 @@ remove_variables(int brace_level)
                             ptr->argsresult = varptr->next;
                         else
                             prevvar->next = varptr->next;
+                        free(varptr);
                     }
                 }
             }
@@ -375,7 +380,20 @@ struct arguments *argsresult = NULL;
 void
 reset_variables(void)
 {
+    struct arguments *p,
+               *next;
+
+    for (p = argsinsert; p; p = next)
+    {
+        next = p->next;
+        free(p);
+    }
     argsinsert = NULL;
+    for (p = argsresult; p; p = next)
+    {
+        next = p->next;
+        free(p);
+    }
     argsresult = NULL;
 }

@@ -434,6 +452,7 @@ remove_variable_from_list(struct arguments **list, struct variable *var)
             prev->next = p->next;
         else
             *list = p->next;
+        free(p);
     }
 }

--
2.43.5


pgsql-hackers by date:

Previous
From: "Joel Jacobson"
Date:
Subject: Re: New "raw" COPY format
Next
From: Masahiko Sawada
Date:
Subject: Re: Fix for consume_xids advancing XIDs incorrectly