Обсуждение: Avoid resource leak (src/test/regress/pg_regress.c)
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
Вложения
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
Вложения
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
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
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/
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