Re: kernel32: Correct WideCharToMultiByte and MultiByteToWideChar error codes and conditions

[Nikolay Sivov]

On 5/10/2012 00:52, Alex Henrie wrote:
> This patch was a lot harder to figure out than I thought it was going to be.
>
> The MSDN documentation of most of these behaviors can be found at
> http://msdn.microsoft.com/en-us/library/windows/desktop/dd319072(v=vs.85).as...
> Does any application depend on that?
>
> ---
>   dlls/kernel32/locale.c       |   38 ++++++++++++++---
>   dlls/kernel32/tests/locale.c |   94 ++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 126 insertions(+), 6 deletions(-)
> This is a wrong file to place tests into.
>
> diff --git a/dlls/kernel32/tests/locale.c b/dlls/kernel32/tests/locale.c
> index 3216ef9..9cbb124 100644
> --- a/dlls/kernel32/tests/locale.c
> +++ b/dlls/kernel32/tests/locale.c
> @@ -3137,6 +3137,99 @@ static void test_IdnToUnicode(void)
>       }
>   }
>
> +static void test_WideCharToMultiByte_error(UINT page, DWORD flags, LPCWSTR src,
> +                                           INT srclen, LPSTR dst, INT dstlen,
> +                                           LPCSTR defchar, BOOL* used,
> +                                           const char* test_description,
> +                                           DWORD expected_error)
> +{
> +    SetLastError(ERROR_SUCCESS);
> +    WideCharToMultiByte(page, flags, src, srclen, dst, dstlen, defchar, used);
> +    ok(GetLastError() == expected_error,
> +       "codepage %i, %s, expected error %i got error %i\n", page,
> +       test_description, expected_error, GetLastError());
> +}
> +
> +static void test_MultiByteToWideChar_error(UINT page, DWORD flags, LPCSTR src,
> +                                           INT srclen, LPWSTR dst, INT dstlen,
> +                                           const char* test_description,
> +                                           DWORD expected_error)
> +{
> +    SetLastError(ERROR_SUCCESS);
> +    MultiByteToWideChar(page, flags, src, srclen, dst, dstlen);
> +    ok(GetLastError() == expected_error,
> +       "codepage %i, %s, expected error %i got error %i\n", page,
> +       test_description, expected_error, GetLastError());
> +}
> +
> Looks ugly enough, it's better to place some test data in table and loop 
> through it, if you really want to test that.

kernel32: Correct WideCharToMultiByte and MultiByteToWideChar error codes and conditions

[Alex Henrie]

2012/5/9 Nikolay Sivov <bunglehead(a)gmail.com>:
> Does any application depend on that?

Probably. Better safe than sorry, and Wine's current behavior
definitely doesn't match what MSDN says nor what Windows actually
does.
> >  dlls/kernel32/locale.c       |   38 ++++++++++++++---
> >  dlls/kernel32/tests/locale.c |   94
> > ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 126 insertions(+), 6 deletions(-)
>
> This is a wrong file to place tests into.

Which file should I have used?
> Looks ugly enough, it's better to place some test data in table and loop
> through it, if you really want to test that.

A loop is probably overkill, but I will clean it up some more. Thanks
for the feedback,

-Alex

kernel32: Correct WideCharToMultiByte and MultiByteToWideChar error codes and conditions

[Nikolay Sivov]

On 5/10/2012 07:38, Alex Henrie wrote:
> 2012/5/9 Nikolay Sivov<bunglehead(a)gmail.com>:
> > Does any application depend on that?
> > Probably. Better safe than sorry, and Wine's current behavior
> > definitely doesn't match what MSDN says nor what Windows actually
> > does.
> > > dlls/kernel32/locale.c       |   38 ++++++++++++++---
> > >   dlls/kernel32/tests/locale.c |   94
> > > ++++++++++++++++++++++++++++++++++++++++++
> > >   2 files changed, 126 insertions(+), 6 deletions(-)
> > > This is a wrong file to place tests into.
> > > Which file should I have used?
> > > codepage.c, you could search all test files for a thing you're trying to 
> > > add.
> > Looks ugly enough, it's better to place some test data in table and loop
> > through it, if you really want to test that.
> > A loop is probably overkill, but I will clean it up some more. Thanks
> > for the feedback,
> > Nothing really changed in try3, test helpers names are still 
> > meaningless. Also you'll be probably asked to split this, cause your 
> > changes are independent - validation of input parameters and flags 
> > handling are separate fixes.
>
> -Alex

kernel32: Correct WideCharToMultiByte and MultiByteToWideChar error codes and conditions

[Alex Henrie]

2012/5/10 Nikolay Sivov <bunglehead(a)gmail.com>:
> > Which file should I have used?
>
> codepage.c, you could search all test files for a thing you're trying to
> add.

Okay, I've moved the tests to codepage.c in try 4. Thanks for the tip,
I'll keep that in mind from now on.
> Nothing really changed in try3, test helpers names are still meaningless.

Well, what should I name them?
> Also you'll be probably asked to split this, cause your changes are
> independent - validation of input parameters and flags handling are separate
> fixes.

In this case, we're just validating that flags is 0, which is the same
idea as the other parameter checks, so it should be fine.