Skip to content

Commit 2b41de4

Browse files
tglsfdctglsfdc
committed
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/[email protected]
1 parent 85312d9 commit 2b41de4

File tree

7 files changed

+86
-53
lines changed

7 files changed

+86
-53
lines changed

‎src/interfaces/ecpg/preproc/descriptor.c

+10-4
Original file line numberDiff line numberDiff line change
@@ -344,11 +344,17 @@ descriptor_variable(const char *name, int input)
344344
struct variable *
345345
sqlda_variable(const char *name)
346346
{
347-
struct variable *p = (struct variable *) mm_alloc(sizeof(struct variable));
348-
349-
p->name = mm_strdup(name);
350-
p->type = (struct ECPGtype *) mm_alloc(sizeof(struct ECPGtype));
347+
/*
348+
* Presently, sqlda variables are only needed for the duration of the
349+
* current statement. Rather than add infrastructure to manage them,
350+
* let's just loc_alloc them.
351+
*/
352+
struct variable *p = (struct variable *) loc_alloc(sizeof(struct variable));
353+
354+
p->name = loc_strdup(name);
355+
p->type = (struct ECPGtype *) loc_alloc(sizeof(struct ECPGtype));
351356
p->type->type = ECPGt_sqlda;
357+
p->type->type_name = NULL;
352358
p->type->size = NULL;
353359
p->type->struct_sizeof = NULL;
354360
p->type->u.element = NULL;

‎src/interfaces/ecpg/preproc/ecpg.addons

+17-30
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ ECPG: rule stmt ViewStmt
135135

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

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

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

257-
if (ptr->connection)
258-
connection = mm_strdup(ptr->connection);
258+
update_connection(ptr->connection);
259259
if (@2[0] == ':')
260260
@$ = cat2_str(@1, "$0");
261261
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
265265
ECPG: addon fetch_args ALL opt_from_in cursor_name
266266
struct cursor *ptr = add_additional_variables(@3, false);
267267

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

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

294-
if (ptr->connection)
295-
connection = mm_strdup(ptr->connection);
292+
update_connection(ptr->connection);
296293
if (@4[0] == ':')
297294
@$ = cat_str(4, @1, @2, @3, "$0");
298295
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
302299
struct cursor *ptr = add_additional_variables(@4, false);
303300
bool replace = false;
304301

305-
if (ptr->connection)
306-
connection = mm_strdup(ptr->connection);
302+
update_connection(ptr->connection);
307303
if (@4[0] == ':')
308304
{
309305
@4 = "$0";
@@ -412,8 +408,7 @@ ECPG: block ClosePortalStmt CLOSE cursor_name
412408
{
413409
if (strcmp(@2, ptr->name) == 0)
414410
{
415-
if (ptr->connection)
416-
connection = mm_strdup(ptr->connection);
411+
update_connection(ptr->connection);
417412
break;
418413
}
419414
}
@@ -483,8 +478,7 @@ ECPG: rule FetchStmt MOVE fetch_args
483478
const char *cursor_marker = @3[0] == ':' ? "$0" : @3;
484479
struct cursor *ptr = add_additional_variables(@3, false);
485480

486-
if (ptr->connection)
487-
connection = mm_strdup(ptr->connection);
481+
update_connection(ptr->connection);
488482

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

496-
if (ptr->connection)
497-
connection = mm_strdup(ptr->connection);
490+
update_connection(ptr->connection);
498491

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

506-
if (ptr->connection)
507-
connection = mm_strdup(ptr->connection);
499+
update_connection(ptr->connection);
508500

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

516-
if (ptr->connection)
517-
connection = mm_strdup(ptr->connection);
508+
update_connection(ptr->connection);
518509

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

526-
if (ptr->connection)
527-
connection = mm_strdup(ptr->connection);
517+
update_connection(ptr->connection);
528518

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

536-
if (ptr->connection)
537-
connection = mm_strdup(ptr->connection);
526+
update_connection(ptr->connection);
538527

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

546-
if (ptr->connection)
547-
connection = mm_strdup(ptr->connection);
535+
update_connection(ptr->connection);
548536

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

556-
if (ptr->connection)
557-
connection = mm_strdup(ptr->connection);
544+
update_connection(ptr->connection);
558545

559546
@$ = cat_str(2, "move backward from", cursor_marker);
560547
}

‎src/interfaces/ecpg/preproc/ecpg.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ bool autocommit = false,
2020
regression_mode = false,
2121
auto_prepare = false;
2222

23+
static const char *progname;
2324
char *output_filename;
2425

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

‎src/interfaces/ecpg/preproc/ecpg.header

+16-1
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ struct variable no_indicator = {"no_indicator", &ecpg_no_indicator, 0, NULL};
7171
static struct ECPGtype ecpg_query = {ECPGt_char_variable, NULL, NULL, NULL, {NULL}, 0};
7272

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

7576

7677
/*
@@ -558,12 +559,26 @@ check_declared_list(const char *name)
558559
{
559560
if (connection && strcmp(ptr->connection, connection) != 0)
560561
mmerror(PARSE_ERROR, ET_WARNING, "connection %s is overwritten with %s by DECLARE statement %s", connection, ptr->connection, name);
561-
connection = mm_strdup(ptr->connection);
562+
update_connection(ptr->connection);
562563
return true;
563564
}
564565
}
565566
return false;
566567
}
568+
569+
/*
570+
* If newconn isn't NULL, update the global "connection" variable to that;
571+
* otherwise do nothing.
572+
*/
573+
static void
574+
update_connection(const char *newconn)
575+
{
576+
if (newconn)
577+
{
578+
free(connection);
579+
connection = mm_strdup(newconn);
580+
}
581+
}
567582
%}
568583

569584
%expect 0

‎src/interfaces/ecpg/preproc/ecpg.trailer

+18-13
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ CreateAsStmt: CREATE OptTemp TABLE create_as_target AS
7777

7878
at: AT connection_object
7979
{
80+
if (connection)
81+
free(connection);
8082
connection = mm_strdup(@2);
8183

8284
/*
@@ -560,13 +562,12 @@ type_declaration: S_TYPEDEF
560562
var_declaration:
561563
storage_declaration var_type
562564
{
563-
actual_type[struct_level].type_storage = mm_strdup(@1);
565+
actual_type[struct_level].type_storage = loc_strdup(@1);
564566
actual_type[struct_level].type_enum = $2.type_enum;
565-
actual_type[struct_level].type_str = mm_strdup($2.type_str);
566-
actual_type[struct_level].type_dimension = mm_strdup($2.type_dimension);
567-
actual_type[struct_level].type_index = mm_strdup($2.type_index);
568-
actual_type[struct_level].type_sizeof =
569-
$2.type_sizeof ? mm_strdup($2.type_sizeof) : NULL;
567+
actual_type[struct_level].type_str = $2.type_str;
568+
actual_type[struct_level].type_dimension = $2.type_dimension;
569+
actual_type[struct_level].type_index = $2.type_index;
570+
actual_type[struct_level].type_sizeof = $2.type_sizeof;
570571

571572
actual_startline[struct_level] = hashline_number();
572573
}
@@ -576,13 +577,12 @@ var_declaration:
576577
}
577578
| var_type
578579
{
579-
actual_type[struct_level].type_storage = mm_strdup("");
580+
actual_type[struct_level].type_storage = loc_strdup("");
580581
actual_type[struct_level].type_enum = $1.type_enum;
581-
actual_type[struct_level].type_str = mm_strdup($1.type_str);
582-
actual_type[struct_level].type_dimension = mm_strdup($1.type_dimension);
583-
actual_type[struct_level].type_index = mm_strdup($1.type_index);
584-
actual_type[struct_level].type_sizeof =
585-
$1.type_sizeof ? mm_strdup($1.type_sizeof) : NULL;
582+
actual_type[struct_level].type_str = $1.type_str;
583+
actual_type[struct_level].type_dimension = $1.type_dimension;
584+
actual_type[struct_level].type_index = $1.type_index;
585+
actual_type[struct_level].type_sizeof = $1.type_sizeof;
586586

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

877-
$$.type_str = (this->type->type_enum == ECPGt_varchar || this->type->type_enum == ECPGt_bytea) ? mm_strdup("") : mm_strdup(this->name);
877+
$$.type_str = (this->type->type_enum == ECPGt_varchar || this->type->type_enum == ECPGt_bytea) ? "" : this->name;
878878
$$.type_enum = this->type->type_enum;
879879
$$.type_dimension = this->type->type_dimension;
880880
$$.type_index = this->type->type_index;
@@ -1008,6 +1008,7 @@ s_struct_union_symbol: SQL_STRUCT symbol
10081008
{
10091009
$$.su = "struct";
10101010
$$.symbol = @2;
1011+
free(ECPGstruct_sizeof);
10111012
ECPGstruct_sizeof = mm_strdup(cat_str(3, "sizeof(",
10121013
cat2_str($$.su, $$.symbol),
10131014
")"));
@@ -1021,6 +1022,7 @@ s_struct_union_symbol: SQL_STRUCT symbol
10211022

10221023
s_struct_union: SQL_STRUCT
10231024
{
1025+
free(ECPGstruct_sizeof);
10241026
ECPGstruct_sizeof = mm_strdup(""); /* This must not be NULL to
10251027
* distinguish from simple types. */
10261028
@$ = "struct";
@@ -1700,18 +1702,21 @@ ECPGVar: SQL_VAR
17001702
ECPGWhenever: SQL_WHENEVER SQL_SQLERROR action
17011703
{
17021704
when_error.code = $3.code;
1705+
free(when_error.command);
17031706
when_error.command = $3.command ? mm_strdup($3.command) : NULL;
17041707
@$ = cat_str(3, "/* exec sql whenever sqlerror ", $3.str, "; */");
17051708
}
17061709
| SQL_WHENEVER NOT SQL_FOUND action
17071710
{
17081711
when_nf.code = $4.code;
1712+
free(when_nf.command);
17091713
when_nf.command = $4.command ? mm_strdup($4.command) : NULL;
17101714
@$ = cat_str(3, "/* exec sql whenever not found ", $4.str, "; */");
17111715
}
17121716
| SQL_WHENEVER SQL_SQLWARNING action
17131717
{
17141718
when_warn.code = $3.code;
1719+
free(when_warn.command);
17151720
when_warn.command = $3.command ? mm_strdup($3.command) : NULL;
17161721
@$ = cat_str(3, "/* exec sql whenever sql_warning ", $3.str, "; */");
17171722
}

‎src/interfaces/ecpg/preproc/output.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,11 @@ output_statement(const char *stmt, int whenever_mode, enum ECPG_statement_type s
155155

156156
/* dump variables to C file */
157157
dump_variables(argsinsert, 1);
158+
argsinsert = NULL;
158159
fputs("ECPGt_EOIT, ", base_yyout);
159160
dump_variables(argsresult, 1);
161+
argsresult = NULL;
160162
fputs("ECPGt_EORT);", base_yyout);
161-
reset_variables();
162163

163164
whenever_action(whenever_mode | 2);
164165
}

0 commit comments

Comments
 (0)