Обсуждение: pg_getaddrinfo_all() with hintp=NULL
Hi!
I'm trying to use pg_getaddrinfo_all() with NULL hints, but got segfault.
According to man(3) getaddrinfo, hints may be passed as NULL. In this case af_family
is equivalent to AF_UNSPEC. I'm adding a patch to pg_getaddrinfo_all() to make it available by passing hintp as NULL.
--
With best regards,
Sergey Tatarintsev,
PostgresPro
Вложения
> On 10 Nov 2025, at 06:14, Sergey Tatarintsev <s.tatarintsev@postgrespro.ru> wrote: > I'm trying to use pg_getaddrinfo_all() with NULL hints, but got segfault. > According to man(3) getaddrinfo, hints may be passed as NULL. In this case af_family > is equivalent to AF_UNSPEC. Right, but pg_getaddrinfo_all isn't getaddrinfo and doesn't claim to be. Since pg_getaddrinfo_all can return AF_UNIX as opposed to getaddrinfo, it's not clear why hints == NULL should mean ipv4|ipv6 here. Accepting NULL or also (subtly) breaks the API for pg_freeaddrinfo_all which is defined to take ai_family from the hints passed to pg_getaddrinfo_all. Now, reading the code makes it obvious that it will work anyways, but at the very least a patch to accept a NULL hint should update the function comment for pg_freeaddrinfo_all. -- Daniel Gustafsson
11.11.2025 23:23, Daniel Gustafsson пишет: >> On 10 Nov 2025, at 06:14, Sergey Tatarintsev <s.tatarintsev@postgrespro.ru> wrote: >> I'm trying to use pg_getaddrinfo_all() with NULL hints, but got segfault. >> According to man(3) getaddrinfo, hints may be passed as NULL. In this case af_family >> is equivalent to AF_UNSPEC. > Right, but pg_getaddrinfo_all isn't getaddrinfo and doesn't claim to be. Since > pg_getaddrinfo_all can return AF_UNIX as opposed to getaddrinfo, it's not clear > why hints == NULL should mean ipv4|ipv6 here. > > Accepting NULL or also (subtly) breaks the API for pg_freeaddrinfo_all which is > defined to take ai_family from the hints passed to pg_getaddrinfo_all. Now, > reading the code makes it obvious that it will work anyways, but at the very > least a patch to accept a NULL hint should update the function comment for > pg_freeaddrinfo_all. > > -- > Daniel Gustafsson > Daniel, thanks for review! I added a comment to pg_freeaddrinfo_all. I don't think it made sense to comment that hint_ai_family simply shouldn't be equal to AF_UNIX, so I specified that if the original hints were NULL, hint_ai_family should be equal to AF_UNSPEC. -- With best regards, Sergey Tatarintsev, PostgresPro
Вложения
> On 12 Nov 2025, at 09:46, Sergey Tatarintsev <s.tatarintsev@postgrespro.ru> wrote: > I added a comment to pg_freeaddrinfo_all. > I don't think it made sense to comment that hint_ai_family simply shouldn't be equal to AF_UNIX, > so I specified that if the original hints were NULL, hint_ai_family should be equal to AF_UNSPEC. Agreed. It's still not clear to ne that we should make pg_getaddr_info() mimic the API of getaddrinfo since it's not a thin wrapper over getaddrinfo. The alternative option would be to return EAI_FAIL on a null hint and force the caller to provide an AF_UNSPEC if that is what is indeed asked for. -- Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
> It's still not clear to ne that we should make pg_getaddr_info() mimic the API
> of getaddrinfo since it's not a thin wrapper over getaddrinfo. The alternative
> option would be to return EAI_FAIL on a null hint and force the caller to
> provide an AF_UNSPEC if that is what is indeed asked for.
We already reject null hints (by crashing), and have done for decades.
I don't think there's anything very wrong with the current code, and I
don't really see that the proposed patch is an improvement. If
anything, I'd just add a comment to pg_getaddrinfo_all saying that
unlike bare getaddrinfo, we require a valid hintp.
regards, tom lane
> On 13 Nov 2025, at 01:17, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Daniel Gustafsson <daniel@yesql.se> writes: >> It's still not clear to ne that we should make pg_getaddr_info() mimic the API >> of getaddrinfo since it's not a thin wrapper over getaddrinfo. The alternative >> option would be to return EAI_FAIL on a null hint and force the caller to >> provide an AF_UNSPEC if that is what is indeed asked for. > > We already reject null hints (by crashing), and have done for decades. > I don't think there's anything very wrong with the current code, and I > don't really see that the proposed patch is an improvement. If > anything, I'd just add a comment to pg_getaddrinfo_all saying that > unlike bare getaddrinfo, we require a valid hintp. That's my thinking as well, unless objected to I will apply the below which use the same phrasing as the comment on pg_getnameinfo_all. diff --git a/src/common/ip.c b/src/common/ip.c index 0e7897a5c8f..71e5934557e 100644 --- a/src/common/ip.c +++ b/src/common/ip.c @@ -48,6 +48,9 @@ static int getnameinfo_unix(const struct sockaddr_un *sa, int salen, /* * pg_getaddrinfo_all - get address info for Unix, IPv4 and IPv6 sockets + * + * The API of this routine differs from the standard getaddrinfo() definition + * in that it requires a valid hintp, a null pointer is not allowed. */ int pg_getaddrinfo_all(const char *hostname, const char *servname, -- Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
> That's my thinking as well, unless objected to I will apply the below which use
> the same phrasing as the comment on pg_getnameinfo_all.
WFM.
regards, tom lane
> On 13 Nov 2025, at 16:26, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Daniel Gustafsson <daniel@yesql.se> writes: >> That's my thinking as well, unless objected to I will apply the below which use >> the same phrasing as the comment on pg_getnameinfo_all. > > WFM. Thanks, done. -- Daniel Gustafsson