Re: msvcrt: portable printf integer conversions

[Mike McCormack]

Hi Jesse,

Jesse Allen wrote:
> Here's a patch to start the work on getting our own printf number
> conversions done internally.  It adds pf_integer_conv, which is
> capable of handling d,x,o,u,i.  However, a large part of my intent is
> to add support for I64 size integers.  So I've decided to only foward
> I64 sizes to it at this time, and leave the rest intact, as I've only
> tested I64.  I sent a message to wine-devel last night for comments,
> but it seems to have gotten lost.  We need to guage how to restructure
> the printf code, especially since I've found two bugs in our current
> implementation.

Code to handle %I64 and %I32 has been missing for a while.  Thanks for 
looking into this.

Please make sure to write some test cases, as changing it will probably 
break something (or even make some of the current test cases pass).

If you look at glibc's printf code, you'll realize how messy complete 
printf handling is...  IMO, it would be better to let glibc handle the 
complexities of printf where possible, and work around the differences 
as we have done so far.

Some comments on the patch:
> - if( flags->Format == 'X' )
> -
>    digits = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> - else
> -
>    digits = "0123456789abcdefghijklmnopqrstuvwxyz";

Do you really need the whole alphabet?
> -
>            else if( *(p+1) == '3' && *(p+2) == '2' )
> -
>            {
> -
>                FIXME("%%I32 unhandled\n");
> -
>                p += 3;
> -
>            }
> -
>            else
> -
>            {
> -
>                FIXME("%%I unhandled\n");
> -
>                p++;

Seems like if you have the code to do %I64, then doing %I32 would be 
easy too...

I think that you'll find that %I31 is invalid, but again you'll need a 
test case to make sure.

Mike

msvcrt: portable printf integer conversions

[Jesse Allen]

On 9/26/05, Mike McCormack  wrote:
> Hi Jesse,
>
> Code to handle %I64 and %I32 has been missing for a while.  Thanks for
> looking into this.
>
> Please make sure to write some test cases, as changing it will probably
> break something (or even make some of the current test cases pass).
>
> If you look at glibc's printf code, you'll realize how messy complete
> printf handling is...  IMO, it would be better to let glibc handle the
> complexities of printf where possible, and work around the differences
> as we have done so far.
>
> Some comments on the patch:
> > - if( flags->Format == 'X' )
> > -
> >    digits = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> > - else
> > -
> >    digits = "0123456789abcdefghijklmnopqrstuvwxyz";
>
> Do you really need the whole alphabet?

Heh,
I've read various printf implementation over time.  The last one,
ReactOS, seemed to provide support for numbering systems base 2
through 36.  You'll see my version does mimic theirs, but we never do
anything other than 8, 10, and 16.  I'm not sure if there is any
printf out there that actually allows you to specify the base.  For
this patch it could easily be cut to the 'f' as maximum, except I use
the 'x' digit in the case of the special 0x and 0X prepending cases. 
To change that, we just add in a test for the large type there.
> > -
> >            else if( *(p+1) == '3' && *(p+2) == '2' )
> > -
> >            {
> > -
> >                FIXME("%%I32 unhandled\n");
> > -
> >                p += 3;
> > -
> >            }
> > -
> >            else
> > -
> >            {
> > -
> >                FIXME("%%I unhandled\n");
> > -
> >                p++;
>
> Seems like if you have the code to do %I64, then doing %I32 would be
> easy too...

Exactly, although we could forward I32 to libc's right?
> I think that you'll find that %I31 is invalid, but again you'll need a
> test case to make sure.
>
> Mike

I believe what happens if you pass a number other that 32 or 64, the
conversion stops.  Note that I64 floats will be just processed like
integers?  Not a big deal as we lacked any real function before.

I will pull up my patch of test cases (never applied) from last time
and see if it will still work against cvs.

Jesse

msvcrt: portable printf integer conversions

[Mike McCormack]

Jesse Allen wrote:
> I've read various printf implementation over time.  The last one,
> ReactOS, seemed to provide support for numbering systems base 2
> through 36.  You'll see my version does mimic theirs, but we never do
> anything other than 8, 10, and 16.  I'm not sure if there is any
> printf out there that actually allows you to specify the base.  For
> this patch it could easily be cut to the 'f' as maximum, except I use
> the 'x' digit in the case of the special 0x and 0X prepending cases. 
> To change that, we just add in a test for the large type there.

Generally if something isn't used, then it shouldn't be included.  You 
specified your 'x' as digits[33], and you could easily delete the rest 
of the alphabet after 'f' and use digits[16] for 'x'.
> I will pull up my patch of test cases (never applied) from last time
> and see if it will still work against cvs.

Somebody (not you, i think) submitted a few test cases before that did 
something like:

printf(buf,"%s %d %c\n", "blah", 10, 'x' );
ok( strlen(buf) == 9, "wrong length\n" );

That doesn't test the code so well, so please make sure to check the 
output is correct in an unambiguous way. eg.

ok( !strcmp(buf,"blah 10 x"), "wrong output\n");

I'd check some combinations of stuff like:

( %I64x %0I64x %1I64d %i64x %i64d %I64D %50I64d %-1I64d %-50I64d etc )
for values: 0, -1, ~1, ~0, 100, -100

Mike