This is the mail archive of the
newlib@sourceware.org
mailing list for the newlib project.
Re: [PATCH] New functions wprintf, fwprintf, swprintf, vwprintf, ?vfwprintf, vswprintf
On Mar 5 23:36, Eric Blake wrote:
> 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?
None of the functions in wchar.h. is guarded that way. I rather keep it
as it is, unless somebody does it for all of wchar.h.
> > +++ 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?
Done.
> > 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/
Fixed.
> > +RETURNS
> > +On success, <<swprintf>> return the number of bytes in
>
> s/bytes/wide characters/
Fixed.
> > +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/
Fixed.
> > +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.
Fixed.
> > 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?
Reverted to EOF. We know that it's -1.
> > +#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?
No. A target could deliberately not support multibyte charsets, only
singlebyte but nevertheless support wide char functions. That's why
you see _MB_CAPABLE in a couple of wc functions. It simplifies the
code extremly for small targets.
> > +#endif /* _MB_CAPABLE */
> > + if (prec >= 0) {
> > + /*
> > + * can't use strlen; can only look for the
>
> s/strlen/wcslen/
Fixed.
> > +_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?
Good idea! I did that in the patch which I applied.
Thanks for your review,
Corinna
--
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat
- References:
- [PATCH] New functions wprintf, fwprintf, swprintf, vwprintf, vfwprintf, vswprintf
- Re: [PATCH] New functions wprintf, fwprintf, swprintf, vwprintf, vfwprintf, vswprintf