Re: [PATCH] msvcrt/tests: Add tests for mbstowcs and wcstombs with a negative count.

[Piotr Caban]

Hi Alex,

On 06/12/16 22:07, Alex Henrie wrote:
> +    ret = mbstowcs(wOut, mSimple, -1);
> +todo_wine
> +    ok(ret == -1 || broken(ret == 4) /* xp */, "mbstowcs returned %i instead of -1\n", ret);
> This will cause compilation warning:
> ../../../../wine_src/dlls/msvcrt/tests/string.c:1863:5: warning: format 
> '%i' expects argument of type 'int', but argument 3 has type 'size_t' 
> [-Wformat=]
>       ok(ret == -1 || broken(ret == 4) /* xp */, "mbstowcs returned %i 
> instead of -1\n", ret);

I'm also not sure if Windows XP behavior should be marked as broken. For 
me newer systems behavior is broken (doesn't handle strings longer then 
INT_MAX). Do you have an application that depends on it?

Thanks,
Piotr

[PATCH] msvcrt/tests: Add tests for mbstowcs and wcstombs with a negative count.

[Alex Henrie]

2016-06-13 4:21 GMT-06:00 Piotr Caban <piotr.caban(a)gmail.com>:
> On 06/12/16 22:07, Alex Henrie wrote:
> > +    ret = mbstowcs(wOut, mSimple, -1);
> > +todo_wine
> > +    ok(ret == -1 || broken(ret == 4) /* xp */, "mbstowcs returned %i
> > instead of -1\n", ret);
>
> This will cause compilation warning:
> ../../../../wine_src/dlls/msvcrt/tests/string.c:1863:5: warning: format '%i'
> expects argument of type 'int', but argument 3 has type 'size_t' [-Wformat=]
>      ok(ret == -1 || broken(ret == 4) /* xp */, "mbstowcs returned %i
> instead of -1\n", ret);

Good point. I will rewrite this to just say "mbstowcs did not return
-1\n", similar to the surrounding tests.
> I'm also not sure if Windows XP behavior should be marked as broken. For me
> newer systems behavior is broken (doesn't handle strings longer then
> INT_MAX). Do you have an application that depends on it?

I discovered this while trying to figure out what's wrong with the
Emergency 3 Demo: https://bugs.winehq.org/show_bug.cgi?id=40775

The bug would be "fixed" if we adopted the Windows XP behavior for
mbstowcs. However, this application works fine on both Windows XP and
Windows 7, so I think that there is actually something else going on
here.

I really don't know which behavior should be considered "correct".
Windows XP's behavior matches the C specification, but Vista and later
all consistently do something else. Maybe we should make Wine behave
differently too depending on which version of Windows is selected in
winecfg, and put "ret == 4 || ret == -1" in the tests, omitting the
broken() macro. What do you think?

-Alex

[PATCH] msvcrt/tests: Add tests for mbstowcs and wcstombs with a negative count.

[Alex Henrie]

2016-06-13 8:56 GMT-06:00 Alex Henrie <alexhenrie24(a)gmail.com>:
> 2016-06-13 4:21 GMT-06:00 Piotr Caban <piotr.caban(a)gmail.com>:
> > I'm also not sure if Windows XP behavior should be marked as broken. For me
> > newer systems behavior is broken (doesn't handle strings longer then
> > INT_MAX). Do you have an application that depends on it?
>
> I discovered this while trying to figure out what's wrong with the
> Emergency 3 Demo: https://bugs.winehq.org/show_bug.cgi?id=40775
>
> The bug would be "fixed" if we adopted the Windows XP behavior for
> mbstowcs. However, this application works fine on both Windows XP and
> Windows 7, so I think that there is actually something else going on
> here.
>
> I really don't know which behavior should be considered "correct".
> Windows XP's behavior matches the C specification, but Vista and later
> all consistently do something else. Maybe we should make Wine behave
> differently too depending on which version of Windows is selected in
> winecfg, and put "ret == 4 || ret == -1" in the tests, omitting the
> broken() macro. What do you think?

I just ran some more tests, and it turns out that the behavior of
Vista and later is even weirder than I thought. On these systems,
mbstowcs with a negative count writes one null byte to the output
string before returning -1, but wcstombs returns -1 without writing
anything.

I think I'd rather have the Windows XP / standard C behavior than the
inconsistent, nonconformant Vista behavior.

-Alex

[PATCH] msvcrt/tests: Add tests for mbstowcs and wcstombs with a negative count.

[Piotr Caban]

On 06/14/16 06:18, Alex Henrie wrote:
> I just ran some more tests, and it turns out that the behavior of
> Vista and later is even weirder than I thought. On these systems,
> mbstowcs with a negative count writes one null byte to the output
> string before returning -1, but wcstombs returns -1 without writing
> anything.
>
> I think I'd rather have the Windows XP / standard C behavior than the
> inconsistent, nonconformant Vista behavior.

I've moved your tests to msvcr90. The behavior is consistent there on 
all windows versions. I think it makes the tests easier to read. I've 
also changed the implementation so all tests are passing.

Thank you,
Piotr

[PATCH] msvcrt/tests: Add tests for mbstowcs and wcstombs with a negative count.

[Alex Henrie]

2016-06-14 4:12 GMT-06:00 Piotr Caban <piotr.caban(a)gmail.com>:
> On 06/14/16 06:18, Alex Henrie wrote:
> > I just ran some more tests, and it turns out that the behavior of
> > Vista and later is even weirder than I thought. On these systems,
> > mbstowcs with a negative count writes one null byte to the output
> > string before returning -1, but wcstombs returns -1 without writing
> > anything.
> >
> > I think I'd rather have the Windows XP / standard C behavior than the
> > inconsistent, nonconformant Vista behavior.
>
> I've moved your tests to msvcr90. The behavior is consistent there on all
> windows versions. I think it makes the tests easier to read. I've also
> changed the implementation so all tests are passing.

It looks like you chose the Windows Vista behavior. That's fine, at
least we match some version of Windows now.

Interestingly, somehow this change fixed the Emergency 3 Demo. That's
a pleasant surprise, I thought that the problem was going to be more
complicated :-D

-Alex