Horton
Menu

Thread

  1. Avoid resource leak (src/test/regress/pg_regress.c)

    Ranier Vilela <ranier.vf@gmail.com> — 2025-10-24T00:37:21Z

    Hi.

Per Coverity.

The function *config_sspi_auth* is responsible for
rewrite pg_hba.conf and pg_ident.conf to use SSPI authentication.

Coverity complains that the struct addrinfo gai_result is leaked.
The variable is declared inside block and is not used
outside the block.

So if the function WSAStartup is successful then the function getaddrinfo
allocates and fills the struct addrinfo.

The memory must be released at the end of the block.

Trivial patch attached.

best regards,
Ranier Vilela

  2. Re: Avoid resource leak (src/test/regress/pg_regress.c)

    Michael Paquier <michael@paquier.xyz> — 2025-10-24T06:03:27Z

    On Thu, Oct 23, 2025 at 09:37:21PM -0300, Ranier Vilela wrote:
> The function *config_sspi_auth* is responsible for
> rewrite pg_hba.conf and pg_ident.conf to use SSPI authentication.
> 
> Coverity complains that the struct addrinfo gai_result is leaked.
> The variable is declared inside block and is not used
> outside the block.
> 
> So if the function WSAStartup is successful then the function getaddrinfo
> allocates and fills the struct addrinfo.
> 
> The memory must be released at the end of the block.

Not sure that this one is worth caring about.  We have a bunch of
allocations that we know would be freed once a binary exits.  This is
just one of them, allocated in the context of what is a short-term
execution.
--
Michael

  3. Re: Avoid resource leak (src/test/regress/pg_regress.c)

    Kirill Reshke <reshkekirill@gmail.com> — 2025-10-24T06:06:18Z

    On Fri, 24 Oct 2025, 11:03 Michael Paquier, <michael@paquier.xyz> wrote:

> On Thu, Oct 23, 2025 at 09:37:21PM -0300, Ranier Vilela wrote:
> > The function *config_sspi_auth* is responsible for
> > rewrite pg_hba.conf and pg_ident.conf to use SSPI authentication.
> >
> > Coverity complains that the struct addrinfo gai_result is leaked.
> > The variable is declared inside block and is not used
> > outside the block.
> >
> > So if the function WSAStartup is successful then the function getaddrinfo
> > allocates and fills the struct addrinfo.
> >
> > The memory must be released at the end of the block.
>
> Not sure that this one is worth caring about.  We have a bunch of
> allocations that we know would be freed once a binary exits.  This is
> just one of them, allocated in the context of what is a short-term
> execution.
> --
> Michael
>

Hi!
Yes, this is indeed minor and false positive, but maybe there is still
value in committing this - to silence coverity? There is nothing wrong in
being extra-tidy about memory

>

  4. Re: Avoid resource leak (src/test/regress/pg_regress.c)

    Ranier Vilela <ranier.vf@gmail.com> — 2025-10-24T11:13:04Z

    Em sex., 24 de out. de 2025 às 03:03, Michael Paquier <michael@paquier.xyz>
escreveu:

> On Thu, Oct 23, 2025 at 09:37:21PM -0300, Ranier Vilela wrote:
> > The function *config_sspi_auth* is responsible for
> > rewrite pg_hba.conf and pg_ident.conf to use SSPI authentication.
> >
> > Coverity complains that the struct addrinfo gai_result is leaked.
> > The variable is declared inside block and is not used
> > outside the block.
> >
> > So if the function WSAStartup is successful then the function getaddrinfo
> > allocates and fills the struct addrinfo.
> >
> > The memory must be released at the end of the block.
>
> Not sure that this one is worth caring about.  We have a bunch of
> allocations that we know would be freed once a binary exits.

Fortunately, this happens less and less.


>   This is
> just one of them, allocated in the context of what is a short-term
> execution.
>
In this case, I believe pg_regress is run, thousands of times, on hundreds
of computers.

best regards,
Ranier Vilela

  5. Re: Avoid resource leak (src/test/regress/pg_regress.c)

    Álvaro Herrera <alvherre@kurilemu.de> — 2025-10-24T12:21:42Z

    On 2025-Oct-24, Ranier Vilela wrote:

> Em sex., 24 de out. de 2025 às 03:03, Michael Paquier
> <michael@paquier.xyz> escreveu:

> > This is just one of them, allocated in the context of what is a
> > short-term execution.
>
> In this case, I believe pg_regress is run, thousands of times, on
> hundreds of computers.

Yes, but the memory is released at the end of the program execution,
every single one of those times.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

  6. Re: Avoid resource leak (src/test/regress/pg_regress.c)

    Ranier Vilela <ranier.vf@gmail.com> — 2025-10-24T13:03:48Z

    Em sex., 24 de out. de 2025 às 09:21, Álvaro Herrera <alvherre@kurilemu.de>
escreveu:

> On 2025-Oct-24, Ranier Vilela wrote:
>
> > Em sex., 24 de out. de 2025 às 03:03, Michael Paquier
> > <michael@paquier.xyz> escreveu:
>
> > > This is just one of them, allocated in the context of what is a
> > > short-term execution.
> >
> > In this case, I believe pg_regress is run, thousands of times, on
> > hundreds of computers.
>
> Yes, but the memory is released at the end of the program execution,
> every single one of those times.
>
Yeah, for sure.
But not before thousands of tests were carried out.

The allocated memory is useless until the program exits.
According to getaddrinfo documentation:
"The *getaddrinfo*() function allocates and initializes a linked list

       of *addrinfo* structures, one for each network address that matches
       *node* and *service*, subject to any restrictions imposed by *hints*,
       and returns a pointer to the start of the list in *res*.  The items
       in the linked list are linked by the *ai_next* field.

       There are several reasons why the linked list may have more than
       one *addrinfo* structure, including: the network host is multihomed,
       accessible over multiple protocols"

best regards
Ranier Vilela