Re: BUG #15838: [contrib] vacuumlo: schema variable checked for NULL three times
Heikki Linnakangas <hlinnaka@iki.fi>
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: taem@linukz.org
Cc: pgsql-bugs@lists.postgresql.org
Date: 2019-06-07T09:21:07Z
Lists: pgsql-bugs
On 07/06/2019 09:15, PG Bug reporting form wrote:
> vacuumlo() has this (starting on line 239):
>
> if (!schema || !table || !field)
> {
> fprintf(stderr, "%s", PQerrorMessage(conn));
> PQclear(res);
> PQfinish(conn);
> if (schema != NULL)
> PQfreemem(schema);
> if (schema != NULL)
> PQfreemem(table);
> if (schema != NULL)
> PQfreemem(field);
> return -1;
> }
>
> I think this can be replaced with this:
>
> if (!schema || !table || !field)
> {
> fprintf(stderr, "%s", PQerrorMessage(conn));
> PQclear(res);
> PQfinish(conn);
> if (schema != NULL) {
> PQfreemem(schema);
> PQfreemem(table);
> PQfreemem(field);
> }
> return -1;
> }
Hmm. Currently, if allocating 'schema' fails, but allocating 'table' or
'field' succeeds, you leak memory. I'm pretty sure that was intended to be:
if (!schema || !table || !field)
{
fprintf(stderr, "%s", PQerrorMessage(conn));
PQclear(res);
PQfinish(conn);
if (schema != NULL)
PQfreemem(schema);
if (table != NULL)
PQfreemem(table);
if (field != NULL)
PQfreemem(field);
return -1;
}
I'll go fix it that way, thanks for the report!
- Heikki
Commits
-
Fix copy-pasto in freeing memory on error in vacuumlo.
- 99b5ff2eaf4c 9.5.18 landed
- 2558776b4150 9.6.14 landed
- 88ca787b16cc 10.9 landed
- 12a45a20aa25 11.4 landed
- 77b68e3a4f85 12.0 landed