[PATCH] resolv: Track single-request fallback via _res._flags (bug 31476)

[PATCH] resolv: Track single-request fallback via _res._flags (bug 31476)

[Florian Weimer]

This avoids changing _res.options, which inteferes with change
detection as part of automatic reloading of /etc/resolv.conf.

Tested on x86_64-linux-gnu with the existing tests.  Not sure how to
write a good regression test for this.

---
 resolv/res_send.c        | 12 +++++++-----
 resolv/resolv-internal.h |  2 ++
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/resolv/res_send.c b/resolv/res_send.c
index fb0217204a..ea7cf192b2 100644
--- a/resolv/res_send.c
+++ b/resolv/res_send.c
@@ -947,9 +947,11 @@ send_dg(res_state statp,
 		seconds /= statp->nscount;
 	if (seconds <= 0)
 		seconds = 1;
-	bool single_request_reopen = (statp->options & RES_SNGLKUPREOP) != 0;
-	bool single_request = (((statp->options & RES_SNGLKUP) != 0)
-			       | single_request_reopen);
+	bool single_request_reopen = ((statp->options & RES_SNGLKUPREOP)
+				      || (statp->_flags & RES_F_SNGLKUPREOP));
+	bool single_request = ((statp->options & RES_SNGLKUP)
+			       || (statp->_flags & RES_F_SNGLKUP)
+			       || single_request_reopen);
 	int save_gotsomewhere = *gotsomewhere;
 
 	int retval;
@@ -1006,14 +1008,14 @@ send_dg(res_state statp,
 		       have received the first answer.  */
 		    if (!single_request)
 		      {
-			statp->options |= RES_SNGLKUP;
+			statp->_flags |= RES_F_SNGLKUP;
 			single_request = true;
 			*gotsomewhere = save_gotsomewhere;
 			goto retry;
 		      }
 		    else if (!single_request_reopen)
 		      {
-			statp->options |= RES_SNGLKUPREOP;
+			statp->_flags |= RES_F_SNGLKUPREOP;
 			single_request_reopen = true;
 			*gotsomewhere = save_gotsomewhere;
 			__res_iclose (statp, false);
diff --git a/resolv/resolv-internal.h b/resolv/resolv-internal.h
index 24b164f6b5..944af3ee76 100644
--- a/resolv/resolv-internal.h
+++ b/resolv/resolv-internal.h
@@ -26,6 +26,8 @@
 #define RES_F_VC        0x00000001 /* Socket is TCP.  */
 #define RES_F_CONN      0x00000002 /* Socket is connected.  */
 #define RES_F_EDNS0ERR  0x00000004 /* EDNS0 caused errors.  */
+#define RES_F_SNGLKUP	0x00200000 /* Private version of RES_SNGLKUP.  */
+#define RES_F_SNGLKUPREOP 0x00400000 /* Private version of RES_SNGLKUPREOP.  */
 
 /* The structure HEADER is normally aligned on a word boundary.  In
    some code, we need to access this structure when it may be aligned

base-commit: 54c1efdac55b756a4e2ea95590bcc3ba396a6568

Re: [PATCH] resolv: Track single-request fallback via _res._flags (bug 31476)

[DJ Delorie]

Florian Weimer <fweimer@redhat.com> writes:
> This avoids changing _res.options, which inteferes with change
> detection as part of automatic reloading of /etc/resolv.conf.

It changes _flags instead, which is designated internal, but it looks
like the lifetime of _flags vs options is the same - set it once, zero
it on reload.

> diff --git a/resolv/res_send.c b/resolv/res_send.c
> index fb0217204a..ea7cf192b2 100644
> --- a/resolv/res_send.c
> +++ b/resolv/res_send.c
> @@ -947,9 +947,11 @@ send_dg(res_state statp,
>  		seconds /= statp->nscount;
>  	if (seconds <= 0)
>  		seconds = 1;
> -	bool single_request_reopen = (statp->options & RES_SNGLKUPREOP) != 0;
> +	bool single_request_reopen = ((statp->options & RES_SNGLKUPREOP)
> +				      || (statp->_flags & RES_F_SNGLKUPREOP));

Logical || converts result to 0 or not zero, ok.  We only ever test this
in an "zero or not zero" case anyway, so it doesn't matter what the bits
are.

> -	bool single_request = (((statp->options & RES_SNGLKUP) != 0)
> -			       | single_request_reopen);
> +	bool single_request = ((statp->options & RES_SNGLKUP)
> +			       || (statp->_flags & RES_F_SNGLKUP)
> +			       || single_request_reopen);

Ok.

> -			statp->options |= RES_SNGLKUP;
> +			statp->_flags |= RES_F_SNGLKUP;

Ok.

> -			statp->options |= RES_SNGLKUPREOP;
> +			statp->_flags |= RES_F_SNGLKUPREOP;
>  			single_request_reopen = true;

Ok.  There are no other places in this file that set either of these
(single_request or single_request_reopen).

> +#define RES_F_SNGLKUP	0x00200000 /* Private version of RES_SNGLKUP.  */
> +#define RES_F_SNGLKUPREOP 0x00400000 /* Private version of RES_SNGLKUPREOP.  */

Ok.

LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>