From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/11302 Path: news.gmane.org!.POSTED!not-for-mail From: Alexander Monakov Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH v3] Add RES_OPTIONS support for resolv.conf options overriding Date: Tue, 25 Apr 2017 23:49:50 +0300 (MSK) Message-ID: References: <20170425194557.8520-1-stefan.sedich@gmail.com> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Trace: blaine.gmane.org 1493153407 31331 195.159.176.226 (25 Apr 2017 20:50:07 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 25 Apr 2017 20:50:07 +0000 (UTC) User-Agent: Alpine 2.20.13 (LNX 116 2015-12-14) To: musl@lists.openwall.com Original-X-From: musl-return-11317-gllmg-musl=m.gmane.org@lists.openwall.com Tue Apr 25 22:49:59 2017 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1d37PK-0007uy-Sn for gllmg-musl@m.gmane.org; Tue, 25 Apr 2017 22:49:58 +0200 Original-Received: (qmail 25853 invoked by uid 550); 25 Apr 2017 20:50:02 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 25832 invoked from network); 25 Apr 2017 20:50:02 -0000 In-Reply-To: <20170425194557.8520-1-stefan.sedich@gmail.com> Xref: news.gmane.org gmane.linux.lib.musl.general:11302 Archived-At: I see a couple of pre-existing issues in options parsing (not your problem, just questions for Rich): On Tue, 25 Apr 2017, Stefan Sedich wrote: > +void __parse_resolv_opts(struct resolvconf *conf, char *opts) > +{ > + char *p, *z; > + > + p = strstr(opts, "ndots:"); This accepts xndots, _ndots, etc. I think this is undesirable, prefixing a character could be seen by some users as a way to "comment-out" an option without deleting it, and such loose matching lays a trap for them. It also breaks if a valid option ending in 'ndots' appears in the future. > + p = strstr(opts, "timeout:"); > + if (p && (isdigit(p[8]) || p[8]=='.')) { > + p += 8; > + unsigned long x = strtoul(p, &z, 10); Either stroul should be strtod, or accepting p[8]=='.' is pointless. This was introduced in commit d6cb08bcaca4ff1f921375510ca72bccea969c75 that moved this chunk of code from res_msend.c to resolvconf.c and introduced p[8]=='.' check en passant. > @@ -89,5 +96,11 @@ no_resolv_conf: > > conf->nns = nns; > > + char *res_opts_env = NULL; > + if (!libc.secure) res_opts_env = getenv("RES_OPTIONS"); > + if (res_opts_env) { > + __parse_resolv_opts(conf, res_opts_env); > + } This might look slightly cleaner if written as if (!libc.secure) { const char *opts = getenv("RES_OPTIONS"); if (opts) __parse_resolve_opts(conf, opts); } Alexander