Re: BUG #15838: [contrib] vacuumlo: schema variable checked for NULL three times

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