Re: minor leaks in pg_dump (PG tarball 10.6)

Поиск
Список
Период
Сортировка
От Stephen Frost
Тема Re: minor leaks in pg_dump (PG tarball 10.6)
Дата
Msg-id 20181205155918.GF3415@tamriel.snowman.net
обсуждение исходный текст
Ответ на minor leaks in pg_dump (PG tarball 10.6)  (Pavel Raiskup <praiskup@redhat.com>)
Ответы Re: minor leaks in pg_dump (PG tarball 10.6)  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: minor leaks in pg_dump (PG tarball 10.6)  (Pavel Raiskup <praiskup@redhat.com>)
Список pgsql-hackers
Greetings,

* Pavel Raiskup (praiskup@redhat.com) wrote:
> Among other reports (IMO clearly non-issues), I'm sending patch which
> fixes/points to a few resource leaks detected by Coverity that might be
> worth fixing.  If they are not, feel free to ignore this mail.

> diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
> index 8a93ace9fa..475d6dbd73 100644
> --- a/src/bin/pg_dump/dumputils.c
> +++ b/src/bin/pg_dump/dumputils.c
> @@ -95,6 +95,8 @@ buildACLCommands(const char *name, const char *subname, const char *nspname,
>      {
>          if (!parsePGArray(racls, &raclitems, &nraclitems))
>          {
> +            if (aclitems)
> +                free(aclitems);
>              if (raclitems)
>                  free(raclitems);
>              return false;

Yeah, that could be fixed.

> diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
> index d583154fba..731a08c15c 100644
> --- a/src/bin/pg_dump/pg_dump.c
> +++ b/src/bin/pg_dump/pg_dump.c
> @@ -8284,7 +8284,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
>              res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK);
>
>              numDefaults = PQntuples(res);
> -            attrdefs = (AttrDefInfo *) pg_malloc(numDefaults * sizeof(AttrDefInfo));
>
>              for (j = 0; j < numDefaults; j++)
>              {
> @@ -8304,6 +8303,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
>                  if (tbinfo->attisdropped[adnum - 1])
>                      continue;
>
> +                attrdefs = (AttrDefInfo *) pg_malloc(numDefaults * sizeof(AttrDefInfo));
> +

This change doesn't seem to make any sense to me..?  If anything, seems
like we'd end up overallocating memory *after* this change, where we
don't today (though an analyzer tool might complain because we don't
free the memory from it and instead copy the pointer from each of these
items into the tbinfo structure).

Moving the allocation into the loop would also add unnecessary malloc
traffic, so I don't think we should add this.

> @@ -15951,6 +15952,9 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
>                                    tbinfo->attfdwoptions[j]);
>              }
>          }
> +
> +        free(ftoptions);
> +        free(srvname);
>      }

Yes, those could be free'd too.

I'll see about making those changes.

Thanks!

Stephen

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Pavel Raiskup
Дата:
Сообщение: minor leaks in pg_dump (PG tarball 10.6)
Следующее
От: Tom Lane
Дата:
Сообщение: Re: minor leaks in pg_dump (PG tarball 10.6)