BUG #15838: [contrib] vacuumlo: schema variable checked for NULL three times
The following bug has been logged on the website:
Bug reference: 15838
Logged by: Timur Birsh
Email address: taem@linukz.org
PostgreSQL version: 11.2
Operating system: CentOS 7
Description:
Hello,
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;
}
Thanks,
Timur
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
Hello Heikki,
Thanks for your reply.
Yes, my first intention was to report that 'table' and 'field' should be checked also, but I was not sure.
Regards.
07.06.2019, 15:21, "Heikki Linnakangas" <hlinnaka@iki.fi>:
Show quoted text
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