Re: BUG #15838: [contrib] vacuumlo: schema variable checked for NULLthree times - Mailing list 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