pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #15838: [contrib] vacuumlo: schema variable checked for NULL three times
3+ messages / 3 participants
[nested] [flat]
* BUG #15838: [contrib] vacuumlo: schema variable checked for NULL three times
@ 2019-06-07 06:15 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 3+ messages in thread
From: PG Bug reporting form @ 2019-06-07 06:15 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: taem@linukz.org
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
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: BUG #15838: [contrib] vacuumlo: schema variable checked for NULL three times
@ 2019-06-07 09:21 Heikki Linnakangas <hlinnaka@iki.fi>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 3+ messages in thread
From: Heikki Linnakangas @ 2019-06-07 09:21 UTC (permalink / raw)
To: taem@linukz.org; +Cc: pgsql-bugs@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: BUG #15838: [contrib] vacuumlo: schema variable checked for NULL three times
@ 2019-06-07 09:29 Timur Birsh <taem@linukz.org>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 0 replies; 3+ messages in thread
From: Timur Birsh @ 2019-06-07 09:29 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
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>:
> 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
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2019-06-07 09:29 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-06-07 06:15 BUG #15838: [contrib] vacuumlo: schema variable checked for NULL three times PG Bug reporting form <noreply@postgresql.org>
2019-06-07 09:21 ` Heikki Linnakangas <hlinnaka@iki.fi>
2019-06-07 09:29 ` Timur Birsh <taem@linukz.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox