Skip to content

Commit 7213e2f

Browse files
schmiddyschmiddy
committed
Use CLEARPGRES() macro to call PQclear() and set res to NULL.
This simplifies some of the error handling blocks, as now we can unconditionally use this macro without worrying about multiple PQclear() calls causing a double-free(). Per discussion with Daniele.
1 parent b4d8a90 commit 7213e2f

File tree

2 files changed

+25
-24
lines changed

2 files changed

+25
-24
lines changed

‎bin/pg_repack.c

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ repack_all_databases(const char *orderby)
297297
}
298298
}
299299

300-
PQclear(result);
300+
CLEARPGRES(result);
301301
}
302302

303303
/* result is not copied */
@@ -403,7 +403,7 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize)
403403
}
404404
goto cleanup;
405405
}
406-
PQclear(res);
406+
CLEARPGRES(res);
407407

408408
/* Disable statement timeout. */
409409
command("SET statement_timeout = 0", 0, NULL);
@@ -528,8 +528,7 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize)
528528
ret = true;
529529

530530
cleanup:
531-
if (res)
532-
PQclear(res);
531+
CLEARPGRES(res);
533532
disconnect();
534533
termStringInfo(&sql);
535534
return ret;
@@ -554,7 +553,7 @@ apply_log(PGconn *conn, const repack_table *table, int count)
554553
"SELECT repack.repack_apply($1, $2, $3, $4, $5, $6)",
555554
6, params);
556555
result = atoi(PQgetvalue(res, 0, 0));
557-
PQclear(res);
556+
CLEARPGRES(res);
558557

559558
return result;
560559
}
@@ -630,11 +629,10 @@ repack_one_table(const repack_table *table, const char *orderby)
630629
(errcode(E_PG_COMMAND),
631630
errmsg("trigger %s conflicted for %s",
632631
PQgetvalue(res, 0, 0), table->target_name)));
633-
PQclear(res);
634632
have_error = true;
635633
goto cleanup;
636634
}
637-
PQclear(res);
635+
CLEARPGRES(res);
638636

639637
command(table->create_pktype, 0, NULL);
640638
command(table->create_log, 0, NULL);
@@ -658,13 +656,12 @@ repack_one_table(const repack_table *table, const char *orderby)
658656
if (PQresultStatus(res) != PGRES_TUPLES_OK)
659657
{
660658
printf("%s", PQerrorMessage(conn2));
661-
PQclear(res);
662659
have_error = true;
663660
goto cleanup;
664661
}
665662
buffer[0] = '\0';
666663
strncat(buffer, PQgetvalue(res, 0, 0), sizeof(buffer) - 1);
667-
PQclear(res);
664+
CLEARPGRES(res);
668665

669666
/*
670667
* Not using lock_access_share() here since we know that
@@ -718,11 +715,10 @@ repack_one_table(const repack_table *table, const char *orderby)
718715
if (PQresultStatus(res) != PGRES_COMMAND_OK)
719716
{
720717
elog(WARNING, "Error with LOCK TABLE: %s", PQerrorMessage(conn2));
721-
PQclear(res);
722718
have_error = true;
723719
goto cleanup;
724720
}
725-
PQclear(res);
721+
CLEARPGRES(res);
726722
}
727723

728724
/*
@@ -749,11 +745,10 @@ repack_one_table(const repack_table *table, const char *orderby)
749745
{
750746
elog(WARNING, "Unable to allocate vxid, length: %d\n",
751747
PQgetlength(res, 0, 0));
752-
PQclear(res);
753748
have_error = true;
754749
goto cleanup;
755750
}
756-
PQclear(res);
751+
CLEARPGRES(res);
757752

758753
/* Delete any existing entries in the log table now, since we have not
759754
* yet run the CREATE TABLE ... AS SELECT, which will take in all existing
@@ -827,7 +822,7 @@ repack_one_table(const repack_table *table, const char *orderby)
827822
*/
828823
command(index.create_index, 0, NULL);
829824
}
830-
PQclear(res);
825+
CLEARPGRES(res);
831826

832827
/*
833828
* 4. Apply log to temp table until no tuples are left in the log
@@ -858,15 +853,15 @@ repack_one_table(const repack_table *table, const char *orderby)
858853
num_waiting = num;
859854
}
860855

861-
PQclear(res);
856+
CLEARPGRES(res);
862857
sleep(1);
863858
continue;
864859
}
865860
else
866861
{
867862
/* All old transactions are finished;
868863
* go to next step. */
869-
PQclear(res);
864+
CLEARPGRES(res);
870865
break;
871866
}
872867
}
@@ -918,6 +913,7 @@ repack_one_table(const repack_table *table, const char *orderby)
918913
}
919914

920915
cleanup:
916+
CLEARPGRES(res);
921917
termStringInfo(&sql);
922918
if (vxid)
923919
free(vxid);
@@ -959,7 +955,7 @@ kill_ddl(PGconn *conn, Oid relid, bool terminate)
959955
"Canceled %d unsafe queries. Terminating any remaining PIDs.",
960956
PQntuples(res));
961957

962-
PQclear(res);
958+
CLEARPGRES(res);
963959
printfStringInfo(&sql, KILL_COMPETING_LOCKS, relid);
964960
res = pgut_execute(conn, sql.data, 0, NULL);
965961
if (PQresultStatus(res) != PGRES_TUPLES_OK)
@@ -974,7 +970,7 @@ kill_ddl(PGconn *conn, Oid relid, bool terminate)
974970
else
975971
elog(DEBUG2, "No competing DDL to cancel.");
976972

977-
PQclear(res);
973+
CLEARPGRES(res);
978974
termStringInfo(&sql);
979975

980976
return ret;
@@ -1034,21 +1030,21 @@ lock_access_share(PGconn *conn, Oid relid, const char *target_name)
10341030
res = pgut_execute_elevel(conn, sql.data, 0, NULL, DEBUG2);
10351031
if (PQresultStatus(res) == PGRES_COMMAND_OK)
10361032
{
1037-
PQclear(res);
1033+
CLEARPGRES(res);
10381034
break;
10391035
}
10401036
else if (sqlstate_equals(res, SQLSTATE_QUERY_CANCELED))
10411037
{
10421038
/* retry if lock conflicted */
1043-
PQclear(res);
1039+
CLEARPGRES(res);
10441040
pgut_rollback(conn);
10451041
continue;
10461042
}
10471043
else
10481044
{
10491045
/* exit otherwise */
10501046
elog(WARNING, "%s", PQerrorMessage(connection));
1051-
PQclear(res);
1047+
CLEARPGRES(res);
10521048
ret = false;
10531049
break;
10541050
}
@@ -1120,21 +1116,21 @@ lock_exclusive(PGconn *conn, const char *relid, const char *lock_query, bool sta
11201116
res = pgut_execute_elevel(conn, lock_query, 0, NULL, DEBUG2);
11211117
if (PQresultStatus(res) == PGRES_COMMAND_OK)
11221118
{
1123-
PQclear(res);
1119+
CLEARPGRES(res);
11241120
break;
11251121
}
11261122
else if (sqlstate_equals(res, SQLSTATE_QUERY_CANCELED))
11271123
{
11281124
/* retry if lock conflicted */
1129-
PQclear(res);
1125+
CLEARPGRES(res);
11301126
pgut_rollback(conn);
11311127
continue;
11321128
}
11331129
else
11341130
{
11351131
/* exit otherwise */
11361132
printf("%s", PQerrorMessage(connection));
1137-
PQclear(res);
1133+
CLEARPGRES(res);
11381134
ret = false;
11391135
break;
11401136
}

‎bin/pgut/pgut-fe.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,9 @@ extern void pgut_readopt(const char *path, pgut_option options[], int elevel);
7373
extern void pgut_setopt(pgut_option *opt, const char *optarg, pgut_optsrc src);
7474
extern bool pgut_keyeq(const char *lhs, const char *rhs);
7575

76+
/* So we don't need to fret over multiple calls to PQclear(), e.g.
77+
* in cleanup labels.
78+
*/
79+
#define CLEARPGRES(pgres) do { PQclear(pgres); pgres = NULL; } while (0)
80+
7681
#endif /* PGUT_FE_H */

0 commit comments

Comments
 (0)