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
_______________________________________________
freebsd...@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hacke...@freebsd.org"

[Dimitry Andric]

On 21 May 2018, at 04:38, Thomas Munro <mu...@ip9.org> wrote:
>
> 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
can't get it to emit it:

$ 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>

-Dimitry

[Konstantin Belousov]

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]

2018-05-21 15:43 GMT+02:00 Konstantin Belousov <kost...@gmail.com>:
> 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.

--
Ed Schouten <e...@nuxi.nl>
Nuxi, 's-Hertogenbosch, the Netherlands

[Poul-Henning Kamp]

--------
In message <CABh_MKkWOaKZxqye2zW7tGF2...@mail.gmail.com>

, Ed Schouten writes:
>2018-05-21 15:43 GMT+02:00 Konstantin Belousov <kost...@gmail.com>:
>> 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.

--
Poul-Henning Kamp | UNIX since Zilog Zeus 3.20
p...@FreeBSD.ORG | TCP/IP since RFC 956
FreeBSD committer | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

[Konstantin Belousov]

On Tue, May 22, 2018 at 09:37:18AM +0000, Poul-Henning Kamp wrote:
> --------
> In message <CABh_MKkWOaKZxqye2zW7tGF2...@mail.gmail.com>
> , Ed Schouten writes:
> >2018-05-21 15:43 GMT+02:00 Konstantin Belousov <kost...@gmail.com>:
> >> 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]

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.

LGTM.

-Dimitry

[Thomas Munro]

On 22 May 2018 at 22:39, Dimitry Andric <d...@freebsd.org> wrote:
> On 21 May 2018, at 15:43, Konstantin Belousov <kost...@gmail.com> wrote:
>> 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

> 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.