%S %C vfprintf contribution

[Artem B. Bityuckiy]

Hello Jeff!

J. Johnston wrote:

> Artem,
>
>   A number of comments/issues:
>
>   1. The tests for buf size vs MB_LEN_MAX shouldn't be needed.
>      The lowest value is 40 and I don't think I want to see a character
>      set that requires over 40 bytes to encode a single character. :)

I don't think it is an error - it is just paranoid check. Theoretically, 
MB_LEN_MAX may be > 40 (I think only about variable rang - not about 
real multibyte characters). This is preprocessor check an it doesn't 
make much overhead. :-) Please, remove this if you consider necessary.

> 2. Your 'S' code ends up falling into the old strlen check for 's'.  You
>      should prevent this by using an else clause.

Hmm, I think you missed about 'break' at line 728...

>   Your check for cp is
>      NULL should occur at the top just after setting cp.  Your S code can
>      then be an else if and the old 's' code can be in an else clause.

If cp is NULL we consider this as if it was called with %s and set it to 
"(null)". (This means, treat calls vfprintf("%S", NULL) = vfprintf("%s", 
NULL)). In both cases "(null)" string will appead in file stream. Ther 
is a check for cp != NULL at line 678. Is this an error?

>
>   3. Your freeing of the widechar string buffer is not in the right spot.
>      There is an error case that follows allocating the buffer so you 
> won't
>      free it.  

Yes, you are right, I've missed this.

> You should also use a separate ptr other than cp for the
>      malloc'd storage so there is no doubt you are freeing the right 
> thing
>      at the end.

We are setting WIDECHARSTR flag only on line 719 after malloc. I think 
it is no need to introduce another pointer.

>
> -- Jeff J.
>
> Artem B. Bityuckiy wrote:
>
>> Artem B. Bityuckiy wrote:
>>
>>> Hello.
>>>
>>> Nicholas Wourms wrote:
>>> > Artem B. Bityuckiy wrote:
>>> >
>>> >> Hello.
>>> >>
>>> >> Here is the patch that makes vfprintf (and hence, all other
>>> >> vfprintf-based Xprintf functions) understand ISO C 90 %S (same as 
>>> %ls)
>>> >> and %C (same as %lc) format placeholders. Please, review it and give
>>> >> us know if something is wrong.
>>> >>
>>> >> This is tested a little bit- it works and it seems should work with
>>> >> any locale if wcrtomb and wcsrtomb functions work.
>>> >>
>>> >
>>> > I believe the preferred patch type is unidiff or `diff -up`.
>>> OK, here is the patch to vfprintf.c produced by diff -up.
>>> >
>>> > Cheers,
>>> > Nicholas
>>> >
>>> Initially, we've added these changes to Newlib-1.11.0. They was 
>>> tested with Newlib-1.11.0 too. But then I've copy changes to Newlib 
>>> from CVS. vfprintf.c from CVS differs from Newlib-1.11.0's 
>>> vfprintf.c. I didn't test (even compile) Newlib from CVS with these 
>>> patch, but it must work.
>>>
>>> Thanks.
>>>
>>
>
>
I've attached new diff -uN file.

-- 
Best regards,
Artem B. Bityuckiy
SoftMine Corporation, Software Engineer
Tel.: +7(812)329-67-44, +7(812)329-67-45
Mobile: +79112449030
E-mail: abitytsky@softminecorp.com
Web: www.softminecorp.com

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: vfprintf.diff
URL: <http://sourceware.org/pipermail/newlib/attachments/20031107/c63b3794/attachment.ksh>