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

@@ -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));
+	/*
+	 * 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 = mm_strdup(name);
-	p->type = (struct ECPGtype *) mm_alloc(sizeof(struct ECPGtype));
+	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;

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);
 	}

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];
 

src/interfaces/ecpg/preproc/ecpg.header

@@ -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

src/interfaces/ecpg/preproc/ecpg.trailer

@@ -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, "; */");
 	}

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);
 }

src/interfaces/ecpg/preproc/variable.c

@@ -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);
 	}
 }