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
Attachments:
avoid-resource-leak-pg_regress.patchapplication/octet-stream; name=avoid-resource-leak-pg_regress.patchDownload
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index efc41fca2b..c0a747373d 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1038,7 +1038,7 @@ config_sspi_auth(const char *pgdata, const char *superuser_name)
* ::1 (IPv6 loopback) as a numeric host address string.
*/
{
- struct addrinfo *gai_result;
+ struct addrinfo *gai_result = NULL;
struct addrinfo hints;
WSADATA wsaData;
@@ -1053,6 +1053,9 @@ config_sspi_auth(const char *pgdata, const char *superuser_name)
have_ipv6 = (WSAStartup(MAKEWORD(2, 2), &wsaData) == 0 &&
getaddrinfo("::1", NULL, &hints, &gai_result) == 0);
+
+ if (gai_result)
+ freeaddrinfo(gai_result);
}
/* Check a Write outcome and report any error. */
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
Show quoted text
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