diff mbox

Further <resolv.h> cleanup

Message ID b1bd58c8-d3db-de29-5520-b84c2c44008f@redhat.com
State New
Headers show

Commit Message

Florian Weimer April 11, 2017, 10:33 a.m. UTC 
I'm not sure if this is okay from an API compatibility point of view.

_res._flags is supposed to be private to the resolver, so the RES_F_* 
constants should not be in an installed header file, but we nevertheless 
included them in <resolv.h>.

The other cleanups definitions should be uncontroversial.  The function 
declarations were discovered by comparing the symbol lists of libc and 
libresolv with an C AST dump after parsing <resolv.h>.

Thanks,
Florian

Comments

Adhemerval Zanella Netto April 11, 2017, 6:19 p.m. UTC | #1 
On 11/04/2017 07:33, Florian Weimer wrote:
> I'm not sure if this is okay from an API compatibility point of view.
> 
> _res._flags is supposed to be private to the resolver, so the RES_F_* constants should not be in an installed header file, but we nevertheless included them in <resolv.h>.

In a strict way we should continue to provide it (at least on compat way), however
since it is clearly an wrong exposure from glibc and messing with the flag could
lead to unknown side effects I would say this change seems ok.  Also, trying to get
for the flags leads to no meaningful usage on debian codesarch, so I think we should
be safe 

Aas a side note, I also think the effort to keep to maintain a certain level of
api compatibility for this specific issues is just waste effort without much gain. 

> 
> The other cleanups definitions should be uncontroversial.  The function declarations were discovered by comparing the symbol lists of libc and libresolv with an C AST dump after parsing <resolv.h>.
> 
> Thanks,
> Florian
Florian Weimer April 13, 2017, 8:51 a.m. UTC | #2 
On 04/11/2017 08:19 PM, Adhemerval Zanella wrote:
> On 11/04/2017 07:33, Florian Weimer wrote:
>> I'm not sure if this is okay from an API compatibility point of view.
>>
>> _res._flags is supposed to be private to the resolver, so the RES_F_* constants should not be in an installed header file, but we nevertheless included them in <resolv.h>.
> 
> In a strict way we should continue to provide it (at least on compat way), however
> since it is clearly an wrong exposure from glibc and messing with the flag could
> lead to unknown side effects I would say this change seems ok.  Also, trying to get
> for the flags leads to no meaningful usage on debian codesarch, so I think we should
> be safe
> 
> Aas a side note, I also think the effort to keep to maintain a certain level of
> api compatibility for this specific issues is just waste effort without much gain.

Thanks, that was my conclusion as well.  I will commit this patch.

Florian
diff mbox

Patch

resolv: Remove internal and unused definitions from <resolv.h>

The RES_F_* constants are only used with the private _res._flags
member.  RES_EXHAUSTIVE is unused.  The removed function
declarations refer to functions not actually exported by glibc,
so they are unusable by applications.

2017-04-11  Florian Weimer  <fweimer@redhat.com>

	* resolv/resolv.h (RES_EXHAUSTIVE, p_section, res_npquery)
	(res_nisourserver): Remove definition.
	(p_section, res_npquery, res_nisourserver): Remove declaration.
	(RES_F_VC, RES_F_CONN, RES_F_EDNS0ERR): Move ...
	* resolv/resolv-internal.h (RES_F_VC, RES_F_CONN, RES_F_EDNS0ERR):
	... here.
	* resolv/res_send.c: Include <resolv/resolv-internal.h> instead of
	<resolv.h>.
	* resolv/res_query.c: Likewise.
	* resolv/res_debug.c (p_section): Define as static.

diff --git a/resolv/res_debug.c b/resolv/res_debug.c
index 14557dd..e23559b 100644
--- a/resolv/res_debug.c
+++ b/resolv/res_debug.c
@@ -115,6 +115,8 @@ 
 
 extern const char *_res_sectioncodes[] attribute_hidden;
 
+static const char *p_section(int section, int opcode);
+
 /*
  * Print the current options.
  */
@@ -512,7 +514,7 @@  libresolv_hidden_def (p_type)
 /*
  * Return a string for the type.
  */
-const char *
+static const char *
 p_section(int section, int opcode) {
 	const struct res_sym *symbols;
 
diff --git a/resolv/res_query.c b/resolv/res_query.c
index 07dc6f6..6f3eada 100644
--- a/resolv/res_query.c
+++ b/resolv/res_query.c
@@ -74,6 +74,7 @@ 
 #include <errno.h>
 #include <netdb.h>
 #include <resolv.h>
+#include <resolv/resolv-internal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
diff --git a/resolv/res_send.c b/resolv/res_send.c
index 28c4cab..995e1be 100644
--- a/resolv/res_send.c
+++ b/resolv/res_send.c
@@ -101,7 +101,7 @@ 
 #include <errno.h>
 #include <fcntl.h>
 #include <netdb.h>
-#include <resolv.h>
+#include <resolv/resolv-internal.h>
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/resolv/resolv-internal.h b/resolv/resolv-internal.h
index 99fc17c..d35df1c 100644
--- a/resolv/resolv-internal.h
+++ b/resolv/resolv-internal.h
@@ -22,6 +22,12 @@ 
 #include <resolv.h>
 #include <stdbool.h>
 
+/* Resolver flags.  Used for _flags in struct __res_state.  */
+#define RES_F_VC        0x00000001 /* Socket is TCP.  */
+#define RES_F_CONN      0x00000002 /* Socket is connected.  */
+#define RES_F_EDNS0ERR  0x00000004 /* EDNS0 caused errors.  */
+
+
 /* Internal version of RES_USE_INET6 which does not trigger a
    deprecation warning.  */
 #define DEPRECATED_RES_USE_INET6 0x00002000
diff --git a/resolv/resolv.h b/resolv/resolv.h
index 7809e2e..9fef8e9 100644
--- a/resolv/resolv.h
+++ b/resolv/resolv.h
@@ -163,16 +163,6 @@  struct res_sym {
 };
 
 /*
- * Resolver flags (used to be discrete per-module statics ints).
- */
-#define	RES_F_VC	0x00000001	/* socket is TCP */
-#define	RES_F_CONN	0x00000002	/* socket is connected */
-#define RES_F_EDNS0ERR	0x00000004	/* EDNS0 caused errors */
-
-/* res_findzonecut() options */
-#define	RES_EXHAUSTIVE	0x00000001	/* always do all queries */
-
-/*
  * Resolver options (keep these in synch with res_debug.c, please)
  */
 #define RES_INIT	0x00000001	/* address initialized */
@@ -285,7 +275,6 @@  __END_DECLS
 #define p_fqnname		__p_fqnname
 #define p_option		__p_option
 #define p_secstodate		__p_secstodate
-#define p_section		__p_section
 #define p_time			__p_time
 #define p_type			__p_type
 #define p_rcode			__p_rcode
@@ -299,12 +288,10 @@  __END_DECLS
 #define res_nclose		__res_nclose
 #define res_ninit		__res_ninit
 #define res_nmkquery		__res_nmkquery
-#define res_npquery		__res_npquery
 #define res_nquery		__res_nquery
 #define res_nquerydomain	__res_nquerydomain
 #define res_nsearch		__res_nsearch
 #define res_nsend		__res_nsend
-#define res_nisourserver	__res_nisourserver
 #define res_ownok		__res_ownok
 #define res_queriesmatch	__res_queriesmatch
 #define res_randomid		__res_randomid
@@ -356,14 +343,9 @@  int		res_queriesmatch (const unsigned char *,
 				  const unsigned char *,
 				  const unsigned char *,
 				  const unsigned char *) __THROW;
-const char *	p_section (int __section, int __opcode) __THROW;
 /* Things involving a resolver context. */
 int		res_ninit (res_state) __THROW;
-int		res_nisourserver (const res_state,
-				  const struct sockaddr_in *) __THROW;
 void		fp_resstat (const res_state, FILE *) __THROW;
-void		res_npquery (const res_state, const unsigned char *, int,
-			     FILE *) __THROW;
 const char *	res_hostalias (const res_state, const char *, char *, size_t)
      __THROW;
 int		res_nquery (res_state, const char *, int, int,