farrokhi/pg_repack
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Use CLEARPGRES() macro to call PQclear() and set res to NULL.

Browse Source 
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.
This commit is contained in:
Josh Kupershmidt 2012-12-14 20:13:23 -07:00
parent b4d8a90437
commit 7213e2fe7b
2 changed files with 25 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
bin/pg_repack.c
View File

@ -297,7 +297,7 @@ repack_all_databases(const char *orderby)
} }
} }
PQclear(result); CLEARPGRES(result);
} }
/* result is not copied */ /* result is not copied */
@ -403,7 +403,7 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize)
} }
goto cleanup; goto cleanup;
} }
PQclear(res); CLEARPGRES(res);
/* Disable statement timeout. */ /* Disable statement timeout. */
command("SET statement_timeout = 0", 0, NULL); command("SET statement_timeout = 0", 0, NULL);
@ -528,8 +528,7 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize)
ret = true; ret = true;
cleanup: cleanup:
if (res) CLEARPGRES(res);
PQclear(res);
disconnect(); disconnect();
termStringInfo(&sql); termStringInfo(&sql);
return ret; return ret;
@ -554,7 +553,7 @@ apply_log(PGconn *conn, const repack_table *table, int count)
"SELECT repack.repack_apply($1, $2, $3, $4, $5, $6)", "SELECT repack.repack_apply($1, $2, $3, $4, $5, $6)",
6, params); 6, params);
result = atoi(PQgetvalue(res, 0, 0)); result = atoi(PQgetvalue(res, 0, 0));
PQclear(res); CLEARPGRES(res);
return result; return result;
} }
@ -630,11 +629,10 @@ repack_one_table(const repack_table *table, const char *orderby)
(errcode(E_PG_COMMAND), (errcode(E_PG_COMMAND),
errmsg("trigger %s conflicted for %s", errmsg("trigger %s conflicted for %s",
PQgetvalue(res, 0, 0), table->target_name))); PQgetvalue(res, 0, 0), table->target_name)));
PQclear(res);
have_error = true; have_error = true;
goto cleanup; goto cleanup;
} }
PQclear(res); CLEARPGRES(res);
command(table->create_pktype, 0, NULL); command(table->create_pktype, 0, NULL);
command(table->create_log, 0, NULL); command(table->create_log, 0, NULL);
@ -658,13 +656,12 @@ repack_one_table(const repack_table *table, const char *orderby)
if (PQresultStatus(res) != PGRES_TUPLES_OK) if (PQresultStatus(res) != PGRES_TUPLES_OK)
{ {
printf("%s", PQerrorMessage(conn2)); printf("%s", PQerrorMessage(conn2));
PQclear(res);
have_error = true; have_error = true;
goto cleanup; goto cleanup;
} }
buffer[0] = '\0'; buffer[0] = '\0';
strncat(buffer, PQgetvalue(res, 0, 0), sizeof(buffer) - 1); strncat(buffer, PQgetvalue(res, 0, 0), sizeof(buffer) - 1);
PQclear(res); CLEARPGRES(res);
/* /*
* Not using lock_access_share() here since we know that * Not using lock_access_share() here since we know that
@ -718,11 +715,10 @@ repack_one_table(const repack_table *table, const char *orderby)
if (PQresultStatus(res) != PGRES_COMMAND_OK) if (PQresultStatus(res) != PGRES_COMMAND_OK)
{ {
elog(WARNING, "Error with LOCK TABLE: %s", PQerrorMessage(conn2)); elog(WARNING, "Error with LOCK TABLE: %s", PQerrorMessage(conn2));
PQclear(res);
have_error = true; have_error = true;
goto cleanup; goto cleanup;
} }
PQclear(res); CLEARPGRES(res);
} }
/* /*
@ -749,11 +745,10 @@ repack_one_table(const repack_table *table, const char *orderby)
{ {
elog(WARNING, "Unable to allocate vxid, length: %d\n", elog(WARNING, "Unable to allocate vxid, length: %d\n",
PQgetlength(res, 0, 0)); PQgetlength(res, 0, 0));
PQclear(res);
have_error = true; have_error = true;
goto cleanup; goto cleanup;
} }
PQclear(res); CLEARPGRES(res);
/* Delete any existing entries in the log table now, since we have not /* Delete any existing entries in the log table now, since we have not
* yet run the CREATE TABLE ... AS SELECT, which will take in all existing * 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)
*/ */
command(index.create_index, 0, NULL); command(index.create_index, 0, NULL);
} }
PQclear(res); CLEARPGRES(res);
/* /*
* 4. Apply log to temp table until no tuples are left in the log * 4. Apply log to temp table until no tuples are left in the log
@ -858,7 +853,7 @@ repack_one_table(const repack_table *table, const char *orderby)
num_waiting = num; num_waiting = num;
} }
PQclear(res); CLEARPGRES(res);
sleep(1); sleep(1);
continue; continue;
} }
@ -866,7 +861,7 @@ repack_one_table(const repack_table *table, const char *orderby)
{ {
/* All old transactions are finished; /* All old transactions are finished;
* go to next step. */ * go to next step. */
PQclear(res); CLEARPGRES(res);
break; break;
} }
} }
@ -918,6 +913,7 @@ repack_one_table(const repack_table *table, const char *orderby)
} }
cleanup: cleanup:
CLEARPGRES(res);
termStringInfo(&sql); termStringInfo(&sql);
if (vxid) if (vxid)
free(vxid); free(vxid);
@ -959,7 +955,7 @@ kill_ddl(PGconn *conn, Oid relid, bool terminate)
"Canceled %d unsafe queries. Terminating any remaining PIDs.", "Canceled %d unsafe queries. Terminating any remaining PIDs.",
PQntuples(res)); PQntuples(res));
PQclear(res); CLEARPGRES(res);
printfStringInfo(&sql, KILL_COMPETING_LOCKS, relid); printfStringInfo(&sql, KILL_COMPETING_LOCKS, relid);
res = pgut_execute(conn, sql.data, 0, NULL); res = pgut_execute(conn, sql.data, 0, NULL);
if (PQresultStatus(res) != PGRES_TUPLES_OK) if (PQresultStatus(res) != PGRES_TUPLES_OK)
@ -974,7 +970,7 @@ kill_ddl(PGconn *conn, Oid relid, bool terminate)
else else
elog(DEBUG2, "No competing DDL to cancel."); elog(DEBUG2, "No competing DDL to cancel.");
PQclear(res); CLEARPGRES(res);
termStringInfo(&sql); termStringInfo(&sql);
return ret; return ret;
@ -1034,13 +1030,13 @@ lock_access_share(PGconn *conn, Oid relid, const char *target_name)
res = pgut_execute_elevel(conn, sql.data, 0, NULL, DEBUG2); res = pgut_execute_elevel(conn, sql.data, 0, NULL, DEBUG2);
if (PQresultStatus(res) == PGRES_COMMAND_OK) if (PQresultStatus(res) == PGRES_COMMAND_OK)
{ {
PQclear(res); CLEARPGRES(res);
break; break;
} }
else if (sqlstate_equals(res, SQLSTATE_QUERY_CANCELED)) else if (sqlstate_equals(res, SQLSTATE_QUERY_CANCELED))
{ {
/* retry if lock conflicted */ /* retry if lock conflicted */
PQclear(res); CLEARPGRES(res);
pgut_rollback(conn); pgut_rollback(conn);
continue; continue;
} }
@ -1048,7 +1044,7 @@ lock_access_share(PGconn *conn, Oid relid, const char *target_name)
{ {
/* exit otherwise */ /* exit otherwise */
elog(WARNING, "%s", PQerrorMessage(connection)); elog(WARNING, "%s", PQerrorMessage(connection));
PQclear(res); CLEARPGRES(res);
ret = false; ret = false;
break; break;
} }
@ -1120,13 +1116,13 @@ lock_exclusive(PGconn *conn, const char *relid, const char *lock_query, bool sta
res = pgut_execute_elevel(conn, lock_query, 0, NULL, DEBUG2); res = pgut_execute_elevel(conn, lock_query, 0, NULL, DEBUG2);
if (PQresultStatus(res) == PGRES_COMMAND_OK) if (PQresultStatus(res) == PGRES_COMMAND_OK)
{ {
PQclear(res); CLEARPGRES(res);
break; break;
} }
else if (sqlstate_equals(res, SQLSTATE_QUERY_CANCELED)) else if (sqlstate_equals(res, SQLSTATE_QUERY_CANCELED))
{ {
/* retry if lock conflicted */ /* retry if lock conflicted */
PQclear(res); CLEARPGRES(res);
pgut_rollback(conn); pgut_rollback(conn);
continue; continue;
} }
@ -1134,7 +1130,7 @@ lock_exclusive(PGconn *conn, const char *relid, const char *lock_query, bool sta
{ {
/* exit otherwise */ /* exit otherwise */
printf("%s", PQerrorMessage(connection)); printf("%s", PQerrorMessage(connection));
PQclear(res); CLEARPGRES(res);
ret = false; ret = false;
break; break;
} }

5
bin/pgut/pgut-fe.h
View File

@ -73,4 +73,9 @@ extern void pgut_readopt(const char *path, pgut_option options[], int elevel);
extern void pgut_setopt(pgut_option *opt, const char *optarg, pgut_optsrc src); extern void pgut_setopt(pgut_option *opt, const char *optarg, pgut_optsrc src);
extern bool pgut_keyeq(const char *lhs, const char *rhs); extern bool pgut_keyeq(const char *lhs, const char *rhs);
/* So we don't need to fret over multiple calls to PQclear(), e.g.
* in cleanup labels.
*/
#define CLEARPGRES(pgres) do { PQclear(pgres); pgres = NULL; } while (0)
#endif /* PGUT_FE_H */ #endif /* PGUT_FE_H */