RE: [PATCH] vfwprintf.c

["Yufeng Zhang" <Yufeng dot Zhang at arm dot com>]

Hi Jeff,

Thank you for the effort in verifying the patch. I realized that there
are some problems in the previous patch. Please find the updated patch
in the attachment.

There are two improvement:

1. the new test is more robust: instead of the bogus hard-coded 
wide-char string representation of a floating-point value, the test in
the updated patch calls snprintf and mbstowcs to generate such a
wide-char string to be used by wcscmp to compare with the result from
swprintf.

I tested my previous patch on arm-eabi and the reason it passed on
arm-eabi but failed on x86-linux is because that depending on whether
_NO_LONGDBL is defined or not, either _dtoa_r or _ldtoa_r will be used
for the conversion from floating-point value to char string; while the
latter pads '0' after the significant digits, the former seems to do
some approximation.

2. The previous patch only works if _MB_CAPABLE is not defined; the
updated patch covers the both cases. wcvt is changed to store the full
length of the un-truncated wide-char string in *length, so that its
caller can check whether a larger buffer is needed by comparing the
returned length with the length of the buffer that was passed to wcvt.

The patch has passed the newlib testsuite on arm-eabi and x86-linux.

Please let me know if this updated patch works for you.

Thanks,
Yufeng


2011-06-28  Yufeng Zhang  <yufeng.zhang@arm.com>

        * libc/stdio/vfwprintf.c (wcvt): Add a new parameter len of type
        int.  *length is set to the value of (rev - digits) regardless
        of whether _MB_CAPABLE is defined or not.  Replace BUF with len
        in calling _mbsnrtowcs_r and also in the loop where _MB_CAPABLE
        is not defined.
        (_VFWPRINTF_R): Call wcvt with an extra argument.  Call wcvt
        again with allocated new buffer if buf is not large enough for
        the conversion.
        * testsuite/newlib.stdio/stdio.exp: New.
        * testsuite/newlib.stdio/swprintf.c: Likewise.


> -----Original Message-----
> From: newlib-owner@sourceware.org [mailto:newlib-owner@sourceware.org]
> On Behalf Of Jeff Johnston
> Sent: 10 June 2011 19:39
> To: Yufeng Zhang
> Cc: newlib@sources.redhat.com
> Subject: Re: [PATCH] vfwprintf.c
> 
> Yufeng,
> 
> I'm having trouble verifying this patch.  I think there may possibly be
> another issue in newlib.  When I apply the patch and build both an
> x86-linux version of newlib and mn10300 version of newlib, neither pass
> the test.  They are both padding the result with zeroes.
> 
> Even weirder is that if I change the test to use sprintf, mn10300 gives
> the expected result whereas x86-linux is not.
> 
> I need to look at this a bit more.  It is currently Eclipse release
> train time so I haven't had a long time to look at why my results vary.
> 
> Were you able to verify this worked and what platform did you
> configure/test on?
> 
> -- Jeff J.
> 
> On 06/09/2011 08:50 AM, Yufeng Zhang wrote:
> > Hi,
> >
> > This patch fixes a problem in the newlib vfwprintf implementation.
> >
> > For each conversion specifier in the format string that is [feEgG],
> the
> > current vfwprintf implementation provides only a 40-length wide-char
> > buffer for conversion. This causes code like the following to
> misbehave.
> >
> >    wchar_t largebuf[512];
> >    double val = 1.7976931348623158e+308;
> >    swprintf(largebuf, 512, L"%.*f", 3, val);
> >
> > Only the first 40 characters can be correctly converted due to the
> > limited length of the buffer. This will cause junk in the rest of the
> > converted wide-char sequence, and can potentially corrupt the memory
> > immediately after 'largebuf'.
> >
> > The example code is a repro of the libstdc++-v3 test
> > ./libstdc++-
> v3/testsuite/27_io/basic_ostream/inserters_arithmetic/wchar_t/44
> > 02.cc
> > which currently fails when newlib is used as the C library.
> >
> > The patch was prepared with the consideration of minimum code change.
> > It is probably not worth a bigger surgery in the code since the
> > conversion of extremely long wide char sequence is fairly rare.
> >
> > The 2nd attached patch adds a test to the testsuite.
> >
> > Thanks,
> > Yufeng
> >
> >
> > newlib/ChangeLog
> > 2011-06-09  Yufeng Zhang<yufeng.zhang@arm.com>
> >
> >          * libc/stdio/vfwprintf.c (wcvt): Add new parameter 'len' of
> type
> >          int.  Replace BUF with len.
> >          (VFWPRINTF): Add extra call to wcvt with a malloced new
> buffer,
> >          when the fixed-length buffer 'buf' is not large enough.
>

Attachment: patch-vfwprintf.txt
Description: Text document

Attachment: patch-test.txt
Description: Text document