Adding %m conversion to the printf(3) family

[Roy Marples <roy%marples.name@localhost>]

Hi List

Ours gcc recently grew changes to warn that printf %m is only allowed in 
syslog(3) like functions.
This is problematic because if we compile with -Wmissing-format-attribute gcc 
will warn us that our syslog wrapper function should use sysloglike, but when 
we make the change it when warns us that because of the  functions used 
internally it should be printflike. ie, it cannot make up it's own mind which 
to use.

Instead of trying to make gcc work better, I propose that our libc printf(3) 
family of functions instead support %m. There are sound technical reasons why 
this is a good thing:

1) POSIX syslog(3) requires %m on top of the printf(3) standard
As such printf(3) cannot grow a conflicting %m conversion
printf(3) standard allows for undefined conversions so we're not breaking any 
standard by doing this

2) The alternative of using %s and strerror(3) is problematic in portable 
applications for these reasons
  *  strerror(3) isn't guaranteed to be threadsafe (it is on NetBSD)
  *  strerror_r(3) isn't portable thanks to glibc and others having a different 
signature
  *  strerror_r(3) has a horrible interface anyway as you don't know the size 
of the buffer you need and need to loop through until you get no errors.
  *  strerror_l(3) isn't well supported (recently added to NetBSD but seems to 
be lacking a man page)

3) Code size and binary size are reduced.
Not only is the libc size reduced, but applications like dhcpcd which have a 
syslog(3) wrapper can as well. dhcpcd's binary size reduction is similar to 
libc's at 1.5k on amd64. When dhcpcd first changing it's error reporting to use 
%m instead of strerror, that saved another 2k as well again promoting smaller 
code.

Attached is a patch to demonstrate this, including a man page update to 
clearly state that %m is not recommended in portable applications.

I expect there to be some heated debate over this. To those who disagree with 
this proposal, I expect you to supply a working solution to the problem noted 
at the top of this email. A valid solution could be reverting the gcc change 
and keeping the status quo.

Roy

Index: gen/syslog.c
===================================================================
RCS file: /cvsroot/src/lib/libc/gen/syslog.c,v
retrieving revision 1.54
diff -u -r1.54 syslog.c
--- gen/syslog.c	18 Sep 2014 13:58:20 -0000	1.54
+++ gen/syslog.c	23 Oct 2015 20:56:36 -0000
@@ -213,16 +213,16 @@
 	static const char BRCOSP[] = "]: ";
 	static const char CRLF[] = "\r\n";
 	size_t cnt, prlen, tries;
-	char ch, *p, *t;
+	char *p;
 	struct timeval tv;
 	struct tm tmnow;
 	time_t now;
-	int fd, saved_errno;
+	int fd;
 #define TBUF_LEN	2048
 #define FMT_LEN		1024
 #define MAXTRIES	10
-	char tbuf[TBUF_LEN], fmt_cpy[FMT_LEN], fmt_cat[FMT_LEN] = "";
-	size_t tbuf_left, fmt_left, msgsdlen;
+	char tbuf[TBUF_LEN], fmt_cat[FMT_LEN] = "";
+	size_t tbuf_left, msgsdlen;
 	char *fmt = fmt_cat;
 	int signal_safe = pri & LOG_SIGNAL_SAFE;
 	struct iovec iov[7];	/* prog + [ + pid + ]: + fmt + crlf */
@@ -242,8 +242,6 @@
 	if (!(LOG_MASK(LOG_PRI(pri)) & data->log_mask))
 		return;
 
-	saved_errno = errno;
-
 	/* Set default facility if none specified. */
 	if ((pri & LOG_FACMASK) == 0)
 		pri |= data->log_fac;
@@ -372,42 +370,10 @@
 		strlcat(fmt_cat, msgfmt, FMT_LEN);
 	}
 
-	/*
-	 * We wouldn't need this mess if printf handled %m, or if
-	 * strerror() had been invented before syslog().
-	 */
-	for (t = fmt_cpy, fmt_left = FMT_LEN; (ch = *fmt) != '\0'; ++fmt) {
-		if (ch == '%' && fmt[1] == 'm') {
-			char ebuf[128];
-			++fmt;
-			if (signal_safe ||
-			    strerror_r(saved_errno, ebuf, sizeof(ebuf)))
-				prlen = snprintf_ss(t, fmt_left, "Error %d",
-				    saved_errno);
-			else
-				prlen = snprintf_ss(t, fmt_left, "%s", ebuf);
-			if (prlen >= fmt_left)
-				prlen = fmt_left - 1;
-			t += prlen;
-			fmt_left -= prlen;
-		} else if (ch == '%' && fmt[1] == '%' && fmt_left > 2) {
-			*t++ = '%';
-			*t++ = '%';
-			fmt++;
-			fmt_left -= 2;
-		} else {
-			if (fmt_left > 1) {
-				*t++ = ch;
-				fmt_left--;
-			}
-		}
-	}
-	*t = '\0';
-
 	if (signal_safe)
-		prlen = vsnprintf_ss(p, tbuf_left, fmt_cpy, ap);
+		prlen = vsnprintf_ss(p, tbuf_left, fmt, ap);
 	else
-		prlen = vsnprintf(p, tbuf_left, fmt_cpy, ap);
+		prlen = vsnprintf(p, tbuf_left, fmt, ap);
 
 	if (data->log_stat & (LOG_PERROR|LOG_CONS)) {
 		iov[iovcnt].iov_base = p + msgsdlen;
Index: stdio/printf.3
===================================================================
RCS file: /cvsroot/src/lib/libc/stdio/printf.3,v
retrieving revision 1.64
diff -u -r1.64 printf.3
--- stdio/printf.3	29 Sep 2014 14:58:33 -0000	1.64
+++ stdio/printf.3	23 Oct 2015 20:56:41 -0000
@@ -33,7 +33,7 @@
 .\"
 .\"     @(#)printf.3	8.1 (Berkeley) 6/4/93
 .\"
-.Dd September 29, 2014
+.Dd October 23, 2015
 .Dt PRINTF 3
 .Os
 .Sh NAME
@@ -719,6 +719,13 @@
 .Vt "int *"
 (or variant) pointer argument.
 No argument is converted.
+.It Cm m
+Print the current error message.
+(As denoted by the global variable
+.Va errno ;
+see
+.Xr strerror 3 . )
+No argument is converted.
 .It Cm %
 A
 .Ql %
@@ -836,6 +843,17 @@
 .Fn vsnprintf
 functions conform to
 .St -isoC-99 .
+.Pp
+The
+.Cm \&%m
+conversion format is not standard,
+it is only included here because
+.Xr syslog 3
+requires it.
+Portable programs should use
+.Cm \&%s
+and an argument of
+.Fn strerror errno .
 .Sh HISTORY
 The functions
 .Fn snprintf
Index: stdio/vfwprintf.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdio/vfwprintf.c,v
retrieving revision 1.34
diff -u -r1.34 vfwprintf.c
--- stdio/vfwprintf.c	20 Jan 2014 14:11:03 -0000	1.34
+++ stdio/vfwprintf.c	23 Oct 2015 20:56:44 -0000
@@ -690,6 +690,7 @@
 	int nextarg;		/* 1-based argument index */
 	va_list orgap;		/* original argument pointer */
 	CHAR_T *convbuf;	/* multibyte to wide conversion result */
+	int saved_errno;	/* save errorno for %m */
 
 	/*
 	 * Choose PADSIZE to trade efficiency vs. size.  If larger printf
@@ -822,6 +823,8 @@
 	_DIAGASSERT(fp != NULL);
 	_DIAGASSERT(fmt0 != NULL);
 
+	saved_errno = errno;
+
 	_SET_ORIENTATION(fp, -1);
 
 	thousands_sep = '\0';
@@ -1282,6 +1285,9 @@
 			}
 			break;
 #endif /* !NO_FLOATING_POINT */
+		case 'm':
+			result = (CHAR_T *)strerror(saved_errno);
+			goto print_result;
 		case 'n':
 			/*
 			 * Assignment-like behavior is specified if the
@@ -1353,6 +1359,7 @@
 				}
 			}
 
+print_result:
 			if (prec >= 0) {
 				/*
 				 * can't use STRLEN; can only look for the