Re: [PATCH] Support %m in printf functions

[Eric Blake <eblake at redhat dot com>]

On 01/01/2012 10:59 AM, Yaakov (Cygwin/X) wrote:
> This patch adds support for the "%m" conversion specifier in the printf
> and wprintf functions, a GNU extension:
> 
> http://www.kernel.org/doc/man-pages/online/pages/man3/printf.3.html
> 
> Patch and STC attached.

In addition to the concerns about gating and missing documentation, your
patch has another problem:

>  #endif /* FLOATING_POINT */
> +		case 'm':  /* extension */
> +			cp = _strerror_r (data, data->_errno, 0, NULL);
> +			flags &= ~LONGINT;
> +			goto string;

This should be:

int dummy;
_strerror_r (data, data->_errno, 1, &dummy);

Passing 0 instead of 1 would violate the POSIX requirement that no
library function behaves as if it called strerror().  Passing NULL while
data->_errno is out of range would result in overwriting data->_errno
with EINVAL; whereas you really want two %m designations in the format
string to both print the same incoming errno value.

>  		case 'n':
>  #ifndef _NO_LONGLONG
>  			if (flags & QUADINT)
> @@ -1272,8 +1276,8 @@ reswitch:	switch (ch) {
>  #ifdef _WANT_IO_C99_FORMATS
>  		case 'S':
>  #endif
> -			sign = '\0';
>  			cp = GET_ARG (N, ap, char_ptr_t);
> +string:			sign = '\0';

When you gate things, make sure that this label is also gated, so that
the compiler doesn't warn about an unused label if the gate is off.

>  #endif /* FLOATING_POINT */
> +		case L'm':  /* GNU extension */
> +			cp = (wchar_t *) _strerror_r (data, data->_errno, 0, NULL);
> +			flags &= ~LONGINT;
> +			goto string;

Same problems as above.  Additionally, this looks fishy to cast things
through wchar_t*, although I guess it is no different from how the later
code handles '%s' vs. '%S', and ends up casting the wchar_t* back to
char* before doing mbstate_t conversions.  What an ugly piece of code we
have to work with.

> int
> main (void)
> {
>   double d;
>   printf("%m\n");
>   d = sqrt (-1);  /* EDOM */

Why not just explicitly assign to errno, instead of doing so as a
side-effect of something that required linking with -lm?

>   wprintf(L"%m\n");

I'd modify your test case to use %m twice in the same format string, as
well as play with lengths like %20.10m on one of the cases, to make sure
everything is playing nicely.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature