From: DJ Delorie <dj@redhat.com>
To: Florian Weimer <fweimer@redhat.com>
Cc: libc-alpha@sourceware.org
Subject: Re: [PATCH 3/4] resolv: Support clearing option flags with a “-” prefix (bug 14799)
Date: Thu, 20 Jun 2024 17:06:18 -0400 [thread overview]
Message-ID: <xn5xu3qq7p.fsf@greed.delorie.com> (raw)
In-Reply-To: <53ec8f1ff51463adfee5b761f0c698b8064aa263.1718345824.git.fweimer@redhat.com> (message from Florian Weimer on Fri, 14 Jun 2024 08:20:58 +0200)
Florian Weimer <fweimer@redhat.com> writes:
> I think using a “-” prefix is less confusing than introducing
> double-negation construct (“no-no-tld-query”).
> +* In /etc/resolv.conf and the RES_OPTIONS, option flags can now be
> + prefixed with “-” to clear previously set flags. For example, if
> + /etc/resolv.conf contains “options no-aaaa”, a process running with
> + the RES_OPTIONS=-no-aaaa environment variable performs AAAA DNS
> + queries when the glibc DNS stub resolver is used.
Ok.
> diff --git a/resolv/res_init.c b/resolv/res_init.c
> - uint8_t clear;
Ok, field removed.
> #define STRnLEN(str) str, sizeof (str) - 1
> - { STRnLEN ("rotate"), 0, RES_ROTATE },
> - { STRnLEN ("edns0"), 0, RES_USE_EDNS0 },
> - { STRnLEN ("single-request-reopen"), 0, RES_SNGLKUPREOP },
> - { STRnLEN ("single-request"), 0, RES_SNGLKUP },
> - { STRnLEN ("no_tld_query"), 0, RES_NOTLDQUERY },
> - { STRnLEN ("no-tld-query"), 0, RES_NOTLDQUERY },
> - { STRnLEN ("no-reload"), 0, RES_NORELOAD },
> - { STRnLEN ("use-vc"), 0, RES_USEVC },
> - { STRnLEN ("trust-ad"), 0, RES_TRUSTAD },
> - { STRnLEN ("no-aaaa"), 0, RES_NOAAAA },
> + { STRnLEN ("rotate"), RES_ROTATE },
> + { STRnLEN ("edns0"), RES_USE_EDNS0 },
> + { STRnLEN ("single-request-reopen"), RES_SNGLKUPREOP },
> + { STRnLEN ("single-request"), RES_SNGLKUP },
> + { STRnLEN ("no_tld_query"), RES_NOTLDQUERY },
> + { STRnLEN ("no-tld-query"), RES_NOTLDQUERY },
> + { STRnLEN ("no-reload"), RES_NORELOAD },
> + { STRnLEN ("use-vc"), RES_USEVC },
> + { STRnLEN ("trust-ad"), RES_TRUSTAD },
> + { STRnLEN ("no-aaaa"), RES_NOAAAA },
Ok. No change other than to remove the unneeded 0.
> #define noptions (sizeof (options) / sizeof (options[0]))
> + bool negate_option = *cp == '-';
> + if (negate_option)
> + ++cp;
Ok.
> for (int i = 0; i < noptions; ++i)
> if (strncmp (cp, options[i].str, options[i].len) == 0)
> {
> - if (options[i].clear)
> - parser->template.options &= options[i].flag;
> + if (negate_option)
> + parser->template.options &= ~options[i].flag;
> else
> parser->template.options |= options[i].flag;
Ok.
> diff --git a/resolv/tst-resolv-res_init-skeleton.c b/resolv/tst-resolv-res_init-skeleton.c
> index 6bef62cde2..d3a19eb305 100644
> --- a/resolv/tst-resolv-res_init-skeleton.c
> +++ b/resolv/tst-resolv-res_init-skeleton.c
> @@ -679,6 +679,16 @@ struct test_case test_cases[] =
> "; nameserver[0]: [192.0.2.1]:53\n",
> .res_options = "attempts:5 ndots:3 edns0 ",
> },
> + {.name = "RES_OPTIONS can clear flags",
> + .conf = "options ndots:2 use-vc no-aaaa edns0\n"
> + "nameserver 192.0.2.1\n",
> + .expected = "options ndots:3 use-vc\n"
> + "search example.com\n"
> + "; search[0]: example.com\n"
Where does this search come from? Unrelated to what you're testing, but...
(ok as long as the test succeeds, I suppose :)
> + "nameserver 192.0.2.1\n"
> + "; nameserver[0]: [192.0.2.1]:53\n",
> + .res_options = "ndots:3 -edns0 -no-aaaa",
> + },
Otherwise OK.
LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>
next prev parent reply other threads:[~2024-06-21 0:34 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-14 6:20 [PATCH 0/4] Some DNS stub resolver fixes/enhancements Florian Weimer
2024-06-14 6:20 ` [PATCH 1/4] resolv: Allow short error responses to match any query (bug 31890) Florian Weimer
2024-06-14 7:43 ` Florian Weimer
2024-06-14 9:51 ` Sam James
2024-06-14 10:12 ` Florian Weimer
2024-06-20 20:26 ` DJ Delorie
2024-07-01 13:27 ` Florian Weimer
2024-07-01 20:27 ` DJ Delorie
2024-06-14 6:20 ` [PATCH 2/4] resolv: Do not wait for non-existing second DNS response after error (bug 30081) Florian Weimer
2024-06-20 8:37 ` Andreas Schwab
2024-06-20 20:41 ` DJ Delorie
2024-07-02 7:37 ` Florian Weimer
2024-07-02 16:48 ` DJ Delorie
2024-06-14 6:20 ` [PATCH 3/4] resolv: Support clearing option flags with a “-” prefix (bug 14799) Florian Weimer
2024-06-20 21:06 ` DJ Delorie [this message]
2024-06-14 6:21 ` [PATCH 4/4] resolv: Implement strict-error stub resolver option (bug 27929) Florian Weimer
2024-06-20 21:24 ` DJ Delorie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xn5xu3qq7p.fsf@greed.delorie.com \
--to=dj@redhat.com \
--cc=fweimer@redhat.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).