Re: msvcrt: scanf fix a typo

[James Hawkins]

On Fri, Sep 19, 2008 at 4:39 PM, Austin English <austinenglish(a)gmail.com> wrote:
> Found using -Werror (strangely, didn't occur on ubuntu, but does on
> PC-BSD)...Must be the gcc version.

@@ -63,7 +63,7 @@ static void test_sscanf( void )
     ok( sscanf("1233", "%p", &ptr) == 1, "sscanf failed\n"  );
     ok( ptr == (void *)0x1233,"sscanf reads %p instead of %x\n", ptr, 0x1233 );

-    ok( sscanf("1234", "%P", &ptr) == 1, "sscanf failed\n"  );
+    ok( sscanf("1234", "%p", &ptr) == 1, "sscanf failed\n"  );
     ok( ptr == (void *)0x1234,"sscanf reads %p instead of %x\n", ptr, 0x1234 );

How do you figure that's a typo?  The test right above it is exactly
the same as what you've changed this one to.  I didn't write the test,
but I'm pretty sure the original author meant to test both cases.
This is a good reason why tests should be commented, so the author's
intent is well known.

-- 
James Hawkins

msvcrt: scanf fix a typo

[Austin English]

On Fri, Sep 19, 2008 at 4:51 PM, James Hawkins <truiken(a)gmail.com> wrote:
> On Fri, Sep 19, 2008 at 4:39 PM, Austin English <austinenglish(a)gmail.com> wrote:
> > Found using -Werror (strangely, didn't occur on ubuntu, but does on
> > PC-BSD)...Must be the gcc version.
>
> @@ -63,7 +63,7 @@ static void test_sscanf( void )
>     ok( sscanf("1233", "%p", &ptr) == 1, "sscanf failed\n"  );
>     ok( ptr == (void *)0x1233,"sscanf reads %p instead of %x\n", ptr, 0x1233 );
>
> -    ok( sscanf("1234", "%P", &ptr) == 1, "sscanf failed\n"  );
> +    ok( sscanf("1234", "%p", &ptr) == 1, "sscanf failed\n"  );
>     ok( ptr == (void *)0x1234,"sscanf reads %p instead of %x\n", ptr, 0x1234 );
>
> How do you figure that's a typo?  The test right above it is exactly
> the same as what you've changed this one to.  I didn't write the test,
> but I'm pretty sure the original author meant to test both cases.
> This is a good reason why tests should be commented, so the author's
> intent is well known.
>
> --
> James Hawkins

Relevant code:
    /* check %p with no hex digits */
    ok( sscanf("1233", "%p", &ptr) == 1, "sscanf failed\n"  );
    ok( ptr == (void *)0x1233,"sscanf reads %p instead of %x\n", ptr, 0x1233 );

    ok( sscanf("1234", "%P", &ptr) == 1, "sscanf failed\n"  );
    ok( ptr == (void *)0x1234,"sscanf reads %p instead of %x\n", ptr, 0x1234 );


Comment reads %p. All the other chars are lower case, and you can see
the values are different (1233 vs 1234).

-Austin

msvcrt: scanf fix a typo

[James Hawkins]

On Fri, Sep 19, 2008 at 4:59 PM, Austin English <austinenglish(a)gmail.com> wrote:
> Relevant code:
>    /* check %p with no hex digits */
>    ok( sscanf("1233", "%p", &ptr) == 1, "sscanf failed\n"  );
>    ok( ptr == (void *)0x1233,"sscanf reads %p instead of %x\n", ptr, 0x1233 );
>
> ok( sscanf("1234", "%P", &ptr) == 1, "sscanf failed\n"  );
>    ok( ptr == (void *)0x1234,"sscanf reads %p instead of %x\n", ptr, 0x1234 );
>
> Comment reads %p. All the other chars are lower case, and you can see
> the values are different (1233 vs 1234).

I'm still positive the original author meant to use capital P.
There's no point in adding yet another test for a number that is
different by one digit.

-- 
James Hawkins

msvcrt: scanf fix a typo

[Paul Vriens]

James Hawkins wrote:
> On Fri, Sep 19, 2008 at 4:59 PM, Austin English <austinenglish(a)gmail.com> wrote:
> > Relevant code:
> >    /* check %p with no hex digits */
> >    ok( sscanf("1233", "%p", &ptr) == 1, "sscanf failed\n"  );
> >    ok( ptr == (void *)0x1233,"sscanf reads %p instead of %x\n", ptr, 0x1233 );
> >
> > ok( sscanf("1234", "%P", &ptr) == 1, "sscanf failed\n"  );
> >    ok( ptr == (void *)0x1234,"sscanf reads %p instead of %x\n", ptr, 0x1234 );
> >
> > Comment reads %p. All the other chars are lower case, and you can see
> > the values are different (1233 vs 1234).
>
> I'm still positive the original author meant to use capital P.
> There's no point in adding yet another test for a number that is
> different by one digit.

I have to agree with James here. The original author (Peter Oberndorfer) 
added the tests and an implementation of %p and %P.

Commits:

02fb99e6b360a6f321f716b57df97ca79ec1b9f3
9e3a4652dafbcf1f3f957858a54f2149e91942b7

-- 
Cheers,

Paul.

msvcrt: scanf fix a typo

[Austin English]

On Fri, Sep 19, 2008 at 6:14 PM, Paul Vriens <paul.vriens.wine(a)gmail.com> wrote:
> James Hawkins wrote:
> > On Fri, Sep 19, 2008 at 4:59 PM, Austin English <austinenglish(a)gmail.com>
> > wrote:
> > > Relevant code:
> > >   /* check %p with no hex digits */
> > >   ok( sscanf("1233", "%p", &ptr) == 1, "sscanf failed\n"  );
> > >   ok( ptr == (void *)0x1233,"sscanf reads %p instead of %x\n", ptr,
> > > 0x1233 );
> > >
> > > ok( sscanf("1234", "%P", &ptr) == 1, "sscanf failed\n"  );
> > >   ok( ptr == (void *)0x1234,"sscanf reads %p instead of %x\n", ptr,
> > > 0x1234 );
> > >
> > > Comment reads %p. All the other chars are lower case, and you can see
> > > the values are different (1233 vs 1234).
> >
> > I'm still positive the original author meant to use capital P.
> > There's no point in adding yet another test for a number that is
> > different by one digit.
>
> I have to agree with James here. The original author (Peter Oberndorfer)
> added the tests and an implementation of %p and %P.
>
> Commits:
>
> 02fb99e6b360a6f321f716b57df97ca79ec1b9f3
> 9e3a4652dafbcf1f3f957858a54f2149e91942b7
>
> --
> Cheers,
>
> Paul.

Here's the gcc error:

gcc -c -I. -I. -I../../../include -I../../../include
-I../../../include/msvcrt -I./..  -D_REENTRANT -fPIC -Wall -pipe
-fno-strict-aliasing -Wdeclaration-after-statement -Wwrite-strings
-Wpointer-arith -I/usr/local/include -Werror  -o scanf.o scanf.c
scanf.c: In function `test_sscanf':
scanf.c:66: warning: unknown conversion type character `P' in format
scanf.c:66: warning: too many arguments for format
*** Error code 1

Stop in /usr/home/pcbsd/wine-git/dlls/msvcrt/tests.

msvcrt: scanf fix a typo

[Peter Oberndorfer]

On Samstag 20 September 2008, Paul Vriens wrote:
> James Hawkins wrote:
> > On Fri, Sep 19, 2008 at 4:59 PM, Austin English <austinenglish(a)gmail.com> wrote:
> > > Relevant code:
> > >    /* check %p with no hex digits */
> > >    ok( sscanf("1233", "%p", &ptr) == 1, "sscanf failed\n"  );
> > >    ok( ptr == (void *)0x1233,"sscanf reads %p instead of %x\n", ptr, 0x1233 );
> > >
> > > ok( sscanf("1234", "%P", &ptr) == 1, "sscanf failed\n"  );
> > >    ok( ptr == (void *)0x1234,"sscanf reads %p instead of %x\n", ptr, 0x1234 );
> > >
> > > Comment reads %p. All the other chars are lower case, and you can see
> > > the values are different (1233 vs 1234).
> >
> > I'm still positive the original author meant to use capital P.
> > There's no point in adding yet another test for a number that is
> > different by one digit.
>
> I have to agree with James here. The original author (Peter Oberndorfer) 
> added the tests and an implementation of %p and %P.
>
> Commits:
>
> 02fb99e6b360a6f321f716b57df97ca79ec1b9f3
> 9e3a4652dafbcf1f3f957858a54f2149e91942b7

It was my intention to test lower and upper case %p.
I used different number to make sure this is the result of the current scanf call
not from another call above with the same expected result.
adding ptr = (void*)0xdeadbeef; before each test would have been more explicit.

Greetings Peter

msvcrt: scanf fix a typo

[Juan Lang]

> Relevant code:
>    /* check %p with no hex digits */
>    ok( sscanf("1233", "%p", &ptr) == 1, "sscanf failed\n"  );
>    ok( ptr == (void *)0x1233,"sscanf reads %p instead of %x\n", ptr, 0x1233 );
>
> ok( sscanf("1234", "%P", &ptr) == 1, "sscanf failed\n"  );
>    ok( ptr == (void *)0x1234,"sscanf reads %p instead of %x\n", ptr, 0x1234 );
>
> Comment reads %p. All the other chars are lower case, and you can see
> the values are different (1233 vs 1234).

That's not a compelling argument.  For all we know, the test was
intended to show that both %p and %P are interpreted the same way.
--Juan