From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/6739 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: possible getopt stderr output changes Date: Fri, 19 Dec 2014 16:49:12 -0500 Message-ID: <20141219214912.GC4574@brightrain.aerifal.cx> References: <20141211001032.GA5421@brightrain.aerifal.cx> <20141211220756.GZ4574@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="z6Eq5LdranGa6ru8" X-Trace: ger.gmane.org 1419025780 17670 80.91.229.3 (19 Dec 2014 21:49:40 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 19 Dec 2014 21:49:40 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-6752-gllmg-musl=m.gmane.org@lists.openwall.com Fri Dec 19 22:49:32 2014 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1Y25QO-00086E-Ky for gllmg-musl@m.gmane.org; Fri, 19 Dec 2014 22:49:28 +0100 Original-Received: (qmail 6110 invoked by uid 550); 19 Dec 2014 21:49:27 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 6096 invoked from network); 19 Dec 2014 21:49:26 -0000 Content-Disposition: inline In-Reply-To: <20141211220756.GZ4574@brightrain.aerifal.cx> User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:6739 Archived-At: --z6Eq5LdranGa6ru8 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Dec 11, 2014 at 05:07:56PM -0500, Rich Felker wrote: > On Wed, Dec 10, 2014 at 07:10:32PM -0500, Rich Felker wrote: > > The current getopt code uses some ugly write() sequences to generate > > its output to stderr, and fails to support message translation. The > > latter was an oversight when locale/translation support was added and > > should absolutely be fixed. I'm not sure whether we should leave the > > code using write() though or switch to fprintf. > > It's been pointed out on irc that POSIX requires ferror(stderr) to be > set if writing the message fails. However fwrite could still be used > instead of fprintf. If we need to use stdio at all, however, I'd lean > towards wanting to make the whole write atomic (i.e. hold the lock for > the whole time) which is more of a pain without fprintf. So basically > we're looking at: > > fprintf: > PROS: smaller and simpler code in getopt.c, only one syscall > CONS: additional ~6.5k of additional code pulled in for static > > fwrite: > PROS: minimal static linking deps > CONS: need to use flockfile (or implementation internals) for > atomicity if desired, and multiple writes (so no atomicity on the fd) I'm attaching a patch which allows either solution, and the approach using fwrite is considerably uglier. Even using some stdio internals directly rather than the public API, the resulting getopt.o is 176 bytes larger than the the fprintf version, but I think the ugliness to get the required semantics is the worst part. So I'm strongly leaning towards just using fprintf. The other viable alternative would be having an internal-use function for printing a variadic list of strings atomically through stdio, but I still think there's probably more value in keeping getopt.c independent of musl internals as much as possible. Adding support for translation of the messages is a separate step yet to be done, but the code is setup to support it in either way. Rich --z6Eq5LdranGa6ru8 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="getopt_stdio.diff" diff --git a/src/misc/getopt.c b/src/misc/getopt.c index e77e460..e2a309a 100644 --- a/src/misc/getopt.c +++ b/src/misc/getopt.c @@ -4,6 +4,7 @@ #include #include #include "libc.h" +#include "stdio_impl.h" char *optarg; int optind=1, opterr=1, optopt, __optpos, __optreset=0; @@ -11,6 +12,21 @@ int optind=1, opterr=1, optopt, __optpos, __optreset=0; #define optpos __optpos weak_alias(__optreset, optreset); +#if 0 +static void errmsg(const char *a, const char *b, int l, const char *c) +{ + FILE *f = stderr; + FLOCK(f); + size_t al = strlen(a), bl = strlen(b); + __fwritex((void *)a, al, f) == al && + __fwritex((void *)b, bl, f) == bl && + __fwritex((void *)c, l, f) == l && + __fwritex((void *)"\n", 1, f); + FUNLOCK(f); +} +#define fprintf(fi, fo, a, b, l, c) errmsg(a, b, l, c) +#endif + int getopt(int argc, char * const argv[], const char *optstring) { int i; @@ -66,24 +82,18 @@ int getopt(int argc, char * const argv[], const char *optstring) } while (l && d != c); if (d != c) { - if (optstring[0] != ':' && opterr) { - write(2, argv[0], strlen(argv[0])); - write(2, ": illegal option: ", 18); - write(2, optchar, k); - write(2, "\n", 1); - } + if (optstring[0] != ':' && opterr) + fprintf(stderr, "%s%s%.*s\n", argv[0], + ": illegal option: ", k, optchar); return '?'; } if (optstring[i] == ':') { if (optstring[i+1] == ':') optarg = 0; else if (optind >= argc) { if (optstring[0] == ':') return ':'; - if (opterr) { - write(2, argv[0], strlen(argv[0])); - write(2, ": option requires an argument: ", 31); - write(2, optchar, k); - write(2, "\n", 1); - } + if (opterr) + fprintf(stderr, "%s%s%.*s\n", argv[0], + ": option requires an argument: ", k, optchar); return '?'; } if (optstring[i+1] != ':' || optpos) { --z6Eq5LdranGa6ru8--