Re: [PATCH] New functions wprintf, fwprintf, swprintf, vwprintf, vfwprintf, vswprintf

[Eric Blake <ebb9 at byu dot net>]

Corinna Vinschen <vinschen <at> redhat.com> writes:

> Index: libc/include/wchar.h
> ===================================================================
> +
> +int	_EXFUN(fwprintf, (__FILE *, const wchar_t *, ...));
> +int	_EXFUN(swprintf, (wchar_t *, size_t, const wchar_t *, ...));
> +int	_EXFUN(vfwprintf, (__FILE *, const wchar_t *, __VALIST));
> +int	_EXFUN(vswprintf, (wchar_t *, size_t, const wchar_t *, __VALIST));
> +int	_EXFUN(vwprintf, (const wchar_t *, __VALIST));
> +int	_EXFUN(wprintf, (const wchar_t *, ...));

Missing #ifndef _REENT_ONLY guarding around the non-reentrant versions?

> +++ libc/stdio/Makefile.am	5 Mar 2009 18:01:19 -0000
>  <at>  <at>  -132,7 +132,12  <at>  <at>  ELIX_4_SOURCES = \
>  	putwchar.c		\
>  	ungetwc.c		\
>  	vasniprintf.c		\
> -	vasnprintf.c
> +	vasnprintf.c		\
> +	vwprintf.c		\
> +	swprintf.c		\
> +	vswprintf.c		\
> +	wprintf.c		\
> +	fwprintf.c

Does it make sense to sort this list alphabetically?

> diff -N libc/stdio/swprintf.c
> +
> +        <<swprintf>> is like <<wprintf>>, except that output is directed
> +        to the buffer <[str]>, and the resulting string length is limited
> +	to at most <[size]> bytes, including the terminating <<NUL>>.  As
> +	a special case, if <[size]> is 0, <[str]> can be NULL, and
> +	<<swprintf>> merely calculates how many bytes would be printed.

s/bytes/wide characters/

> +RETURNS
> +On success, <<swprintf>> return the number of bytes in

s/bytes/wide characters/

> +the output string, except the concluding <<NUL>> is not counted.
> +<<wprintf>> and <<fwprintf>> return the number of characters transmitted.
> +
> +If an error occurs, the result of <<wprintf>>, <<fwprintf>>, and
> +<<swprintf>> is a negative value.  For <<wprintf>> and <<fwprintf>>,
> +<<errno>> may be set according to <<fputc>>.  For <<snwprintf>>, <<errno>>

s/fputc/fputwc/

> +may be set to EOVERFLOW if <[size]> or the output length exceeds
> +INT_MAX / sizeof (wchar_t).
> +
> +PORTABILITY
> +POSIX-1.2008
> +
> +#ifdef _HAVE_STDC
> +#include <stdarg.h>
> +#else
> +#include <varargs.h>
> +#endif

By this day and age, _HAVE_STDC should always be set, and we should be able to 
blindly #include <stdarg.h> without worrying about the obsolete <varargs.h>.  
Likewise for all futher uses of _HAVE_STDC in this file.

> Index: libc/stdio/vfwprintf.c
> ===================================================================
> +	/* sorry, fwprintf(read_only_file, "") returns WEOF, not 0 */
> +	if (cantwrite (data, fp)) {
> +		_funlockfile (fp);
> +		return (WEOF);

Are we guaranteed that WEOF is negative?  WEOF is of type wint_t, and it is 
feasible (although I don't know of any systems that do this) that wint_t could 
be unsigned, and that (int)WEOF is not negative.  Since *wprintf return int and 
not wint_t, shouldn't this be an explicit -1 rather than WEOF?

> +#ifdef _MB_CAPABLE
> +			if (ch == L's' && !(flags & LONGINT)) {

By definition, since this is wprintf, shouldn't _MB_CAPABLE be set?  In other 
words, does it buy us anything to make this chunk conditional?

> +#endif /* _MB_CAPABLE */
> +			if (prec >= 0) {
> +				/*
> +				 * can't use strlen; can only look for the

s/strlen/wcslen/

> +
> +_CONST static CH_CLASS chclass[256] = {

Should this and the following state tables be moved into the __ namespace, 
exported, and shared with the character vfprintf for space savings?

-- 
Eric Blake