diff mbox

Fix netdb.h addrinfo namespace (bug 18529)

Message ID alpine.DEB.2.10.1506122147090.17561@digraph.polyomino.org.uk
State New
Headers show

Commit Message

Joseph Myers June 12, 2015, 9:48 p.m. UTC
netdb.h declares interfaces such as getaddrinfo if __USE_POSIX,
i.e. POSIX.1:1990 or later.  However, these interfaces were new in the
2001 edition of POSIX, although the header was in XPG4 and UNIX98, so
they should not be declared for XPG4 or UNIX98.  (This produces
spurious linknamespace test failures, although there are other
failures for this header as well for the same standards so this patch
doesn't remove any XFAILs.)  This patch corrects the condition, and
the conform/ test expectations which were similarly wrong.

Tested for x86_64 and x86 (testsuite, and that installed stripped
shared libraries are unchanged by the patch).

2015-06-12  Joseph Myers  <joseph@codesourcery.com>

	[BZ #18529]
	* resolv/netdb.h [__USE_POSIX]: Change condition to
	[__USE_XOPEN2K].
	* conform/data/netdb.h-data [XPG4 || UNIX98] (struct addrinfo): Do
	not expect.
	[XPG4 || UNIX98] (AI_PASSIVE): Likewise.
	[XPG4 || UNIX98] (AI_CANONNAME): Likewise.
	[XPG4 || UNIX98] (AI_NUMERICHOST): Likewise.
	[XPG4 || UNIX98] (AI_V4MAPPED): Likewise.
	[XPG4 || UNIX98] (AI_ALL): Likewise.
	[XPG4 || UNIX98] (AI_ADDRCONFIG): Likewise.
	[XPG4 || UNIX98] (AI_NUMERICSERV): Likewise.
	[XPG4 || UNIX98] (NI_NOFQDN): Likewise.
	[XPG4 || UNIX98] (NI_NUMERICHOST): Likewise.
	[XPG4 || UNIX98] (NI_NAMEREQD): Likewise.
	[XPG4 || UNIX98] (NI_NUMERICSERV): Likewise.
	[XPG4 || UNIX98] (NI_DGRAM): Likewise.
	[XPG4 || UNIX98] (EAI_AGAIN): Likewise.
	[XPG4 || UNIX98] (EAI_BADFLAGS): Likewise.
	[XPG4 || UNIX98] (EAI_FAIL): Likewise.
	[XPG4 || UNIX98] (EAI_FAMILY): Likewise.
	[XPG4 || UNIX98] (EAI_MEMORY): Likewise.
	[XPG4 || UNIX98] (EAI_NONAME): Likewise.
	[XPG4 || UNIX98] (EAI_SERVICE): Likewise.
	[XPG4 || UNIX98] (EAI_SOCKTYPE): Likewise.
	[XPG4 || UNIX98] (EAI_SYSTEM): Likewise.
	[XPG4 || UNIX98] (EAI_SYSTEM): Likewise.
	[XPG4 || UNIX98] (freeaddrinfo): Likewise.
	[XPG4 || UNIX98] (gai_strerror): Likewise.
	[XPG4 || UNIX98] (getaddrinfo): Likewise.
	[XPG4 || UNIX98] (getnameinfo): Likewise.

Comments

Roland McGrath June 12, 2015, 10:21 p.m. UTC | #1
Again fine but I think the public header should get a comment.
I also think all these user-visible API changes merit NEWS items
(perhaps several of them could be covered by a single item).
Joseph Myers June 12, 2015, 10:44 p.m. UTC | #2
On Fri, 12 Jun 2015, Roland McGrath wrote:

> Again fine but I think the public header should get a comment.

I've made the existing comment refer to POSIX.1:2001 instead of POSIX.1g 
(a reference to POSIX.1:2001 seems the most useful in terms of the 
standards supported by glibc).  I don't think the fact that there was once 
a bug is particularly useful information to go in comments in the header.

> I also think all these user-visible API changes merit NEWS items
> (perhaps several of them could be covered by a single item).

I think of these changes (that do not affect _DEFAULT_SOURCE or 
_GNU_SOURCE and do not obsolete or remove any API) as much like any other 
ordinary bug fix - that is, something for which the entry in the list of 
fixed bugs is sufficient.  If, on review of the changes since 2.21 during 
the freeze for 2.22, it seems these API bug fixes are a significant enough 
group of changes to be worth calling out, they could always be mentioned 
in a general item (but without trying to list details of each individual 
bug that was fixed).
Roland McGrath June 12, 2015, 10:56 p.m. UTC | #3
> I've made the existing comment refer to POSIX.1:2001 instead of POSIX.1g 
> (a reference to POSIX.1:2001 seems the most useful in terms of the 
> standards supported by glibc).  I don't think the fact that there was once 
> a bug is particularly useful information to go in comments in the header.

The "bug" here was an explicit change in the API surface that affects the
build choices of code that is trying to be portable.  So I think it is
appropriate to make clear how the API has evolved over time.

> I think of these changes (that do not affect _DEFAULT_SOURCE or 
> _GNU_SOURCE and do not obsolete or remove any API) as much like any other 
> ordinary bug fix

I don't know why _GNU_SOURCE and _DEFAULT_SOURCE are special in your mind
for this subject.  The API for users of a different feature-test macro
state has changed, so the API has changed.
Joseph Myers June 12, 2015, 11:26 p.m. UTC | #4
On Fri, 12 Jun 2015, Roland McGrath wrote:

> > I've made the existing comment refer to POSIX.1:2001 instead of POSIX.1g 
> > (a reference to POSIX.1:2001 seems the most useful in terms of the 
> > standards supported by glibc).  I don't think the fact that there was once 
> > a bug is particularly useful information to go in comments in the header.
> 
> The "bug" here was an explicit change in the API surface that affects the
> build choices of code that is trying to be portable.  So I think it is
> appropriate to make clear how the API has evolved over time.

There are 113 unsorted conform test XFAILs for header API issues - some 
are bugs in the test expectations, but many are header bugs and there are 
probably many more header bugs hidden by bugs in the test expectations 
that haven't been fully reviewed for most of the relevant standards.  
Only a small proportion of such bugs show up in the linknamespace tests 
rather than the direct tests of header API.  As is often the case with 
bugs, the previous buggy state is not particularly meaningful to describe; 
it's a long series of complicated, nonsensical deviations from the 
standards the headers were meant to be following (and the old API state 
also includes things such as _BSD_SOURCE and _SVID_SOURCE which are no 
longer relevant to glibc and where we've simplified the headers by 
eliminating them).

The high-level description for users is "many headers had bugs where the 
symbols exposed didn't correspond exactly to the standard selected by the 
feature test macros defined, and many such bugs have been fixed over time 
to bring the headers closer to the relevant standards"; something like 
that could go in NEWS for releases with a significant number of such 
fixes.  The place for the detailed information about what each deviation 
from the standards was is Bugzilla and the git history.  I think the 
headers and other sources are better maintained on the basis of describing 
what is, not what was, rather than having large numbers of complicated and 
confusing comments explaining past bugs, save in exceptional cases where 
the history is a basis for current choices (e.g. if something is done in 
an odd way for compatibility with existing binaries).

I think the threshold for calling out such a fix individually in comments 
or NEWS should be quite high, such that the bulk of the many such fixes do 
not reach it.  The threshold for mentioning changes in the documentation 
for individual functions in the manual may be a bit lower - the manual, 
not the headers, should be where the main detailed information for users 
is - but I'd still be inclined to limit it to cases where a program 
written for guarantees about a function in a newer version of glibc could 
quietly fail to work with older versions, not for cases where any failure 
with older glibc would be obvious.

> > I think of these changes (that do not affect _DEFAULT_SOURCE or 
> > _GNU_SOURCE and do not obsolete or remove any API) as much like any other 
> > ordinary bug fix
> 
> I don't know why _GNU_SOURCE and _DEFAULT_SOURCE are special in your mind
> for this subject.  The API for users of a different feature-test macro

Because those are what determine whether the vast bulk of users will 
actually see anything different.
diff mbox

Patch

diff --git a/conform/data/netdb.h-data b/conform/data/netdb.h-data
index c5fd257..63a42ae 100644
--- a/conform/data/netdb.h-data
+++ b/conform/data/netdb.h-data
@@ -44,6 +44,7 @@  macro NO_RECOVERY
 macro TRY_AGAIN
 #endif
 
+#if !defined XPG4 && !defined UNIX98
 type {struct addrinfo}
 element {struct addrinfo} int ai_flags
 element {struct addrinfo} int ai_family
@@ -78,18 +79,23 @@  macro EAI_SERVICE
 macro EAI_SOCKTYPE
 macro EAI_SYSTEM
 macro EAI_OVERFLOW
+#endif
 
 function void endhostent (void)
 function void endnetent (void)
 function void endprotoent (void)
 function void endservent (void)
+#if !defined XPG4 && !defined UNIX98
 function void freeaddrinfo (struct addrinfo*)
 function {const char*} gai_strerror (int)
 function int getaddrinfo (const char*, const char*, const struct addrinfo*, struct addrinfo**)
+#endif
 function {struct hostent*} gethostbyaddr (const void*, socklen_t, int)
 function {struct hostent*} gethostbyname (const char*)
 function {struct hostent*} gethostent (void)
+#if !defined XPG4 && !defined UNIX98
 function int getnameinfo (const struct sockaddr*, socklen_t, char*, socklen_t, char*, socklen_t, int)
+#endif
 function {struct netent*} getnetbyaddr (uint32_t, int)
 function {struct netent*} getnetbyname (const char*)
 function {struct netent*} getnetent (void)
diff --git a/resolv/netdb.h b/resolv/netdb.h
index fe8f3ba..6ff11a1 100644
--- a/resolv/netdb.h
+++ b/resolv/netdb.h
@@ -562,7 +562,7 @@  extern int rresvport_af (int *__alport, sa_family_t __af);
 
 
 /* Extension from POSIX.1g.  */
-#ifdef	__USE_POSIX
+#ifdef __USE_XOPEN2K
 /* Structure to contain information about address of a service provider.  */
 struct addrinfo
 {