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 9 12:14, Howland Craig D (Craig) wrote:
> 1) There's a comment in vfwprintf.c (copied verbatim from vfprintf.c,
> so this is nothing new) regarding the POSIX "'" flag (an extension to
> C99):
>
> /* The ' flag is required by POSIX, but not C99.
> In the C locale, LC_NUMERIC requires
> thousands_sep to be the empty string. And since
> no other locales are supported (yet), this flag
> is currently a no-op. */
>
> Is the statement regarding no other locales still true given the recent
> updates to the locale stuff? That is, could this ' flag be made
> operable now?
It could be done regardless of the state of the locale code. The locale
code doesn't support any locale besides C yet, but the interfaces are
already available. I don't think anything speaks against using
_localeconv_r (data)->thousands_sep right now. Just keep in mind that
the result can be an empty string as well as a multibyte string.
> 2) In the string form of the functions, there's an odd return check,
> which has also been copied from the non-wide starting-point code. For
> example, lines 583-586 of swprintf.c:
>
> ret = _svfwprintf_r (ptr, &f, fmt, ap);
> va_end (ap);
> if (ret < EOF)
> ptr->_errno = EOVERFLOW;
>
> As far as I could see from a brief look through _svfwprintf_r(), it
> cannot
> return anything < EOF. (Nor can _svfprintf_r().) The EOVERFLOW return
> for the conditions described in the docs is handled further up, before
> _svfwprintf_r() is called. Does anyone know what the real purpose of
> this check is? (It is not a simple typo for "if(ret < 0)" assuming that
> the only error return from _svfwprintf_r() is that it ran out of space,
> as _svfwprintf_r() does not return an error if it ran out of space in
> the string. Rather, it returns how many characters would have been put
> into the string while only putting in one less than the max, allowing
> room for '\0' to be appended.)
> I think that it should be modified to be:
>
> ret = _svfwprintf_r (ptr, &f, fmt, ap);
> va_end (ap);
> if (ret >= size)
> ptr->_errno = EOVERFLOW;
>
> (I presently have done this in my local testing copy, along with the
> other changes that I have, but wanted to get a second opinion since the
> original if does not make sense to me.)
Jeff added this check in March 2007 but I don't see any code which
could result in a return value < EOF either. Jeff?
> 3) The documentation says only POSIX-1.2008. Was C99 purposely left
> out?
> I'd think that C99 should be mentioned, and the POSIX extensions to C99
> highlighted in the documentation. (If there's agreement that this
> approach makes sense, I'll do so when I send patches later for the bug
> fixes that I have so far.)
Personally I don't care for C99, I'm only looking for POSIX compatibility.
Feel free to extend the documentation to mention C99.
> 4) Should the POSIX extensions be conditionally able to be left out?
No. Coding conditionalized should be left to the application
programmer, the lib should support all cases.
> FYI, the aforementioned problems that I found and have developed fixes
> for are that the string forms do not correctly put the terminating '\0'
> in (at least not when sizeof(wchar_t) > 1), and that the string forms
> are supposed to return an error if size is exceeded. (The POSIX man
> page fails to mention this part of the C99 standard, but it is still a
> requirement since POSIX explicitly defers to C99.) The former fault
> is probably due to unclear coding in vfprintf.c, so that when porting to
> wide it was not obvious that a wide change was needed. The latter is
> due
> to the oversight (error) in the POSIX manual.
I'm looking forward to the patch.
Thanks,
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
- Re: [PATCH] New functions wprintf, fwprintf, swprintf, vwprintf, ?vfwprintf, vswprintf
- RE: [PATCH] New functions wprintf, fwprintf, swprintf, vwprintf, vfwprintf, vswprintf
- From: Howland Craig D (Craig)