Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Fri, 19 Dec 2014 16:49:12 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: possible getopt stderr output changes

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

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 <limits.h>
 #include <stdlib.h>
 #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) {

Powered by blists - more mailing lists

Your e-mail address:

Powered by Openwall GNU/*/Linux - Powered by OpenVZ