Re: [PATCH] stdio: Add %m

[Jeff Johnston <jjohnstn at redhat dot com>]

There are a number of issues with this patch.

First of all, calling strerror brings in a large number of bytes to the 
load module for an option that is unlikely to be used on newlib 
platforms with space constraints.  Secondly, the patch needs to specify 
__errno_r() rather than errno to account for the reentrancy structure 
passed into __VFPRINTF_R (each reentrancy struct has its own copy of 
errno).  Thirdly, according to ANSI/SUSV3, the strerror() function must 
act as if no library routine calls it.  I see that perror() is currently 
violating that rule, but that means perror() requires changing.  A 
separate buffer and strerror_r should be used instead.  Lastly, I would 
guess that an errno change within printf is not to be reported (i.e. the 
value of __errno_r should probably be saved on entry and cached for 
later %m use).

For the size problem, I propose adding a new flag _WANT_GNUC_EXTENSIONS 
which would help in these situations to determine whether such 
extensions that impact size/performance are really desired.  Similiar 
flags are currently used for removing floating-point, long long, and 
long double support in the printf family.  Alternatively, we can make 
the flag fine-tuned to I/O and call it _WANT_GNUC_IO_EXTENSIONS.

-- Jeff J.

Shaun Jackman wrote:
> This patch to newlib adds the %m format, which outputs
> strerror(errno). This format is a glibc extension. I'm not usually a
> champion of non-standard extensions to any libc. However, this
> particular extension is used fairly widely and is mostly non-invasive.
>
> In particular, busybox uses %m and the maintainers will not accept
> patches converting this format to %s and strerror(errno) due to the
> latter taking more space by making, typically, two additional function
> calls -- one to __errno_location and one to strerror.
>
> Cheers,
> Shaun
>
> 2006-06-05  Shaun Jackman  <sjackman@gmail.com>
>
>     * newlib/libc/stdio/vfprintf.c (_vfprintf_r, _vfiprintf_r): Add
>     the %m format, which outputs strerror(errno). This format is a
>     glibc extension.
>
> Index: newlib/libc/stdio/vfprintf.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/stdio/vfprintf.c,v
> retrieving revision 1.43
> diff -u -r1.43 vfprintf.c
> --- newlib/libc/stdio/vfprintf.c    28 Oct 2005 21:38:59 -0000    1.43
> +++ newlib/libc/stdio/vfprintf.c    5 Jun 2006 16:34:50 -0000
> @@ -905,6 +905,10 @@
>                 sign = '-';
>             break;
> #endif /* FLOATING_POINT */
> +        case 'm': /* glibc extension */
> +            cp = strerror(errno);
> +            size = strlen(cp);
> +            break;
>         case 'n':
> #ifndef _NO_LONGLONG
>             if (flags & QUADINT)
>
>
> ------------------------------------------------------------------------
>
> 2006-06-05  Shaun Jackman  <sjackman@gmail.com>
>
> 	* newlib/libc/stdio/vfprintf.c (_vfprintf_r, _vfiprintf_r): Add
> 	the %m format, which outputs strerror(errno). This format is a
> 	glibc extension.
>
> Index: newlib/libc/stdio/vfprintf.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/stdio/vfprintf.c,v
> retrieving revision 1.43
> diff -u -r1.43 vfprintf.c
> --- newlib/libc/stdio/vfprintf.c	28 Oct 2005 21:38:59 -0000	1.43
> +++ newlib/libc/stdio/vfprintf.c	5 Jun 2006 16:34:50 -0000
> @@ -905,6 +905,10 @@
>  				sign = '-';
>  			break;
>  #endif /* FLOATING_POINT */
> +		case 'm': /* glibc extension */
> +			cp = strerror(errno);
> +			size = strlen(cp);
> +			break;
>  		case 'n':
>  #ifndef _NO_LONGLONG
>  			if (flags & QUADINT)