printf("%m") doesn't generate a warning -- shouldn't it?

[Thomas Munro]

Hi,

As discussed on the PostgreSQL[1] and NetBSD mailing lists[2][3],
syslog-like printf("%m") is a GNU extension that doesn't generate a
warning from Clang or GCC on other operating systems even though when
it doesn't actually work. That's because the __printf__ attribute
that our __printflike macro in /usr/include/stdio.h expands to
effectively means "like printf in glibc, allowing %m", not like POSIX
or our actual libc which just prints out "m" when it sees it.

It'd sure be nice to get a compiler warning on FreeBSD when porting
software that uses that if it doesn't actually work (or ... to support
it).

Concretely, contrib/llvm/tools/clang/include/clang/Analysis/Analyses/FormatString.h
thinks that 'm' is valid but lib/libc/stdio/vfprintf.c doesn't.

Apologies if this was already discussed, it's quite hard to search for
a short squiggly format control string...

Thanks,

[1] https://www.postgresql.org/message-id/2975.1526862605%40sss.pgh.pa.us
[2] https://mail-index.netbsd.org/tech-userlevel/2015/08/21/msg009282.html
[3] https://mail-index.netbsd.org/tech-userlevel/2015/10/23/msg009371.html

[Konstantin Belousov]

Post by Thomas Munro
As discussed on the PostgreSQL[1] and NetBSD mailing lists[2][3],
syslog-like printf("%m") is a GNU extension that doesn't generate a
warning from Clang or GCC on other operating systems even though when
it doesn't actually work. That's because the __printf__ attribute
that our __printflike macro in /usr/include/stdio.h expands to
effectively means "like printf in glibc, allowing %m", not like POSIX
or our actual libc which just prints out "m" when it sees it.
It'd sure be nice to get a compiler warning on FreeBSD when porting
software that uses that if it doesn't actually work (or ... to support
it).

Please submit upstream bug(s) for clang and gcc. It turns out clang has
a -Wformat-non-iso warning flag which should warn about this, but I
$ cat printf-m.c
#include <stdio.h>
int main(void)
{
printf("error: %m\n");
return 0;
}
$ clang -std=c99 -Wformat-non-iso -c printf-m.c
<nothing>

Why not add %m instead ? It is very easy and several people did it in
round-about ways.

diff --git a/lib/libc/stdio/vfprintf.c b/lib/libc/stdio/vfprintf.c
index 29fbd95b399..70f5074e2f7 100644
--- a/lib/libc/stdio/vfprintf.c
+++ b/lib/libc/stdio/vfprintf.c
@@ -317,6 +317,7 @@ __vfprintf(FILE *fp, locale_t locale, const char *fmt0, va_list ap)
int ret; /* return value accumulator */
int width; /* width from format (%8d), or 0 */
int prec; /* precision from format; <0 for N/A */
+ int saved_errno;
char sign; /* sign prefix (' ', '+', '-', or \0) */
struct grouping_state gs; /* thousands' grouping info */

@@ -466,6 +467,7 @@ __vfprintf(FILE *fp, locale_t locale, const char *fmt0, va_list ap)
savserr = fp->_flags & __SERR;
fp->_flags &= ~__SERR;

+ saved_errno = errno;
convbuf = NULL;
fmt = (char *)fmt0;
argtable = NULL;
@@ -776,6 +778,11 @@ reswitch: switch (ch) {
}
break;
#endif /* !NO_FLOATING_POINT */
+ case 'm':
+ cp = strerror(saved_errno);
+ size = (prec >= 0) ? strnlen(cp, prec) : strlen(cp);
+ sign = '\0';
+ break;
case 'n':
/*
* Assignment-like behavior is specified if the

[Ed Schouten]

Post by Konstantin Belousov
Why not add %m instead ? It is very easy and several people did it in
round-about ways.

Added advantage of that approach is that it allows the syslog()
function to be simplified significantly. No need to expand the %m
there, which ends up being quite messy.

[Poul-Henning Kamp]

--------

Post by Ed Schouten

Post by Konstantin Belousov
Why not add %m instead ? It is very easy and several people did it in
round-about ways.

Added advantage of that approach is that it allows the syslog()
function to be simplified significantly. No need to expand the %m
there, which ends up being quite messy.

We have an cleanly expandable printf implementation for exactly
this kind of stuff, so that is not really the case.

However, expandable printf is close to useless when there is no
sane/portable way to teach compilers about new formatting arguments.

[Konstantin Belousov]

Post by Poul-Henning Kamp
--------

Post by Ed Schouten

Post by Konstantin Belousov
Why not add %m instead ? It is very easy and several people did it in
round-about ways.

Added advantage of that approach is that it allows the syslog()
function to be simplified significantly. No need to expand the %m
there, which ends up being quite messy.

We have an cleanly expandable printf implementation for exactly
this kind of stuff, so that is not really the case.
However, expandable printf is close to useless when there is no
sane/portable way to teach compilers about new formatting arguments.

Expandable printf(3) could serve a situation when application wants and
implements an extenstion. It does not work, or is rather weird thing to
try to make it work, when application assumes that the extension is provided
by the operating system.

There was at least one patch which tried to add %m as an extension, and
of course the problem with this approach is that errno may be already
obliterated when the format handler for %m got called.

[Dimitry Andric]

Post by Konstantin Belousov

Post by Thomas Munro
As discussed on the PostgreSQL[1] and NetBSD mailing lists[2][3],
syslog-like printf("%m") is a GNU extension that doesn't generate a
warning from Clang or GCC on other operating systems even though when
it doesn't actually work. That's because the __printf__ attribute
that our __printflike macro in /usr/include/stdio.h expands to
effectively means "like printf in glibc, allowing %m", not like POSIX
or our actual libc which just prints out "m" when it sees it.
It'd sure be nice to get a compiler warning on FreeBSD when porting
software that uses that if it doesn't actually work (or ... to support
it).

Please submit upstream bug(s) for clang and gcc. It turns out clang has
a -Wformat-non-iso warning flag which should warn about this, but I
$ cat printf-m.c
#include <stdio.h>
int main(void)
{
printf("error: %m\n");
return 0;
}
$ clang -std=c99 -Wformat-non-iso -c printf-m.c
<nothing>

Why not add %m instead ? It is very easy and several people did it in
round-about ways.

Sure, that is certainly a nice thing to have, and let's commit it. But
it's still an upstream bug if an explicit warning flag for non-ISO
printf specifiers doesn't work. I'll take that discussion to the LLVM
bug tracker.

Post by Konstantin Belousov
diff --git a/lib/libc/stdio/vfprintf.c b/lib/libc/stdio/vfprintf.c
index 29fbd95b399..70f5074e2f7 100644
--- a/lib/libc/stdio/vfprintf.c
+++ b/lib/libc/stdio/vfprintf.c
@@ -317,6 +317,7 @@ __vfprintf(FILE *fp, locale_t locale, const char *fmt0, va_list ap)
int ret; /* return value accumulator */
int width; /* width from format (%8d), or 0 */
int prec; /* precision from format; <0 for N/A */
+ int saved_errno;
char sign; /* sign prefix (' ', '+', '-', or \0) */
struct grouping_state gs; /* thousands' grouping info */
@@ -466,6 +467,7 @@ __vfprintf(FILE *fp, locale_t locale, const char *fmt0, va_list ap)
savserr = fp->_flags & __SERR;
fp->_flags &= ~__SERR;
+ saved_errno = errno;
convbuf = NULL;
fmt = (char *)fmt0;
argtable = NULL;
@@ -776,6 +778,11 @@ reswitch: switch (ch) {
}
break;
#endif /* !NO_FLOATING_POINT */
+ cp = strerror(saved_errno);
+ size = (prec >= 0) ? strnlen(cp, prec) : strlen(cp);
+ sign = '\0';
+ break;
/*
* Assignment-like behavior is specified if the

LGTM.

-Dimitry

[Thomas Munro]

Post by Dimitry Andric

Post by Konstantin Belousov
Why not add %m instead ? It is very easy and several people did it in
round-about ways.

Sure, that is certainly a nice thing to have, and let's commit it.

+1

Post by Dimitry Andric
But
it's still an upstream bug if an explicit warning flag for non-ISO
printf specifiers doesn't work. I'll take that discussion to the LLVM
bug tracker.

Thanks. Agreed. Also, even if %m is committed, older FreeBSD
releases will be affected for some time, and many other operating
systems are too.