ecpg: clean up some other assorted memory s.
authorTom Lane <[email protected]>
Wed, 27 Nov 2024 17:50:15 +0000 (12:50 -0500)
committerTom Lane <[email protected]>
Wed, 27 Nov 2024 17:50:23 +0000 (12:50 -0500)
Avoid ing 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  allocations.

With this, valgrind reports zero age 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
src/interfaces/ecpg/preproc/ecpg.addons
src/interfaces/ecpg/preproc/ecpg.c
src/interfaces/ecpg/preproc/ecpg.header
src/interfaces/ecpg/preproc/ecpg.trailer
src/interfaces/ecpg/preproc/output.c
src/interfaces/ecpg/preproc/variable.c

index 9b87d07d09fe692d7541931b688404eef74c6e3e..e8c7016bdc11f0a0c0bb276a57d40f8279d79687 100644 (file)
@@ -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;
index 935820c8d464c2036ec57d79977c1df1b7b0cd34..71f7ad26ad8379869984e0934eb3186276c7e017 100644 (file)
@@ -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);
    }
index 73c37631acc6e90721edc6dbf836e34c67b5ba27..2fcc6f8f9967537ca8bd0e98de73ab7ee26940cb 100644 (file)
@@ -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];
 
index e79b6b0de8ca428bc9605608417e05e3879e1a7d..dde69a3969598077a84365b8a0a13ec76c090b5d 100644 (file)
@@ -71,6 +71,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);
 
 
 /*
@@ -558,12 +559,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
index 321bcbccac7b911dee94e9f5cf6c9d9f07a002e6..6f94b832a037f7b8b9377f3eb8ea960670967e26 100644 (file)
@@ -77,6 +77,8 @@ CreateAsStmt: CREATE OptTemp TABLE create_as_target AS
 
 at: AT connection_object
    {
+       if (connection)
+           free(connection);
        connection = mm_strdup(@2);
 
        /*
@@ -560,13 +562,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();
    }
@@ -576,13 +577,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();
    }
@@ -874,7 +874,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;
@@ -1008,6 +1008,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),
                                              ")"));
@@ -1021,6 +1022,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";
@@ -1700,18 +1702,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, "; */");
    }
index a18904f88bd38c48ddd2fdb708c7d6243432af09..b190e9f0cea2a842eadf7b7b4abf83621345e634 100644 (file)
@@ -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);
 }
index e9bdfe56456130f9d407220821331d9849a519b0..ec5afde7d69a6fa5d6d2f283ee918a1b12bdae3e 100644 (file)
@@ -315,10 +315,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 */
@@ -326,10 +328,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 */
@@ -337,6 +341,7 @@ remove_variables(int brace_level)
                            ptr->argsresult = varptr->next;
                        else
                            prevvar->next = varptr->next;
+                       free(varptr);
                    }
                }
            }
@@ -376,7 +381,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;
 }
 
@@ -435,6 +453,7 @@ remove_variable_from_list(struct arguments **list, struct variable *var)
            prev->next = p->next;
        else
            *list = p->next;
+       free(p);
    }
 }