Re: [PATCH 1/3] kernel32: Support UTF-7 in MultiByteToWideChar. (try 2)

[Alexandre Julliard]

Alex Henrie  writes:
> Sorry, I noticed a couple of errors in the utf7_mbstowcs and
> utf7_wcstombs documentation. They are now corrected.

You still haven't addressed the issues I mentioned last time around.

[PATCH 1/3] kernel32: Support UTF-7 in MultiByteToWideChar. (try 2)

[Alex Henrie]

2014-10-07 7:42 GMT-06:00 Alexandre Julliard :
> Alex Henrie  writes:
> > Sorry, I noticed a couple of errors in the utf7_mbstowcs and
> > utf7_wcstombs documentation. They are now corrected.
>
> You still haven't addressed the issues I mentioned last time around.

Did you notice the long list of changes at
https://www.winehq.org/pipermail/wine-patches/2014-October/134808.html
? A minor documentation fix was not the only improvement this time
around.

-Alex

[PATCH 1/3] kernel32: Support UTF-7 in MultiByteToWideChar. (try 2)

[Michael Stefaniuc]

On 10/07/2014 06:06 PM, Alex Henrie wrote:
> 2014-10-07 7:42 GMT-06:00 Alexandre Julliard :
> > Alex Henrie  writes:
> > > Sorry, I noticed a couple of errors in the utf7_mbstowcs and
> > > utf7_wcstombs documentation. They are now corrected.
> >
> > You still haven't addressed the issues I mentioned last time around.
>
> Did you notice the long list of changes at
> https://www.winehq.org/pipermail/wine-patches/2014-October/134808.html
> ? A minor documentation fix was not the only improvement this time
> around.

You changed your patch but did you change the stuff that Alexandre wanted?

bye
    michael

[PATCH 1/3] kernel32: Support UTF-7 in MultiByteToWideChar. (try 2)

[Alex Henrie]

2014-10-07 13:35 GMT-06:00 Michael Stefaniuc :
> On 10/07/2014 06:06 PM, Alex Henrie wrote:
> > 2014-10-07 7:42 GMT-06:00 Alexandre Julliard :
> > > Alex Henrie  writes:
> > > > Sorry, I noticed a couple of errors in the utf7_mbstowcs and
> > > > utf7_wcstombs documentation. They are now corrected.
> > >
> > > You still haven't addressed the issues I mentioned last time around.
> >
> > Did you notice the long list of changes at
> > https://www.winehq.org/pipermail/wine-patches/2014-October/134808.html
> > ? A minor documentation fix was not the only improvement this time
> > around.
>
> You changed your patch but did you change the stuff that Alexandre wanted?

Alexandre stated that he wanted to see more tests for invalid input:
https://www.winehq.org/pipermail/wine-devel/2012-December/098051.html

The new patchset includes many more tests for invalid input, and
writing those tests exposed a bug in utf7_mbstowcs which is now fixed.
The 14 examples in test_utf7_string_conversion add 29 tests which go
through literally hundreds of code paths.

Alexandre: Please, communicate why you are rejecting the code. This
issue will drag on and on until we can come to an agreement.

-Alex

[PATCH 1/3] kernel32: Support UTF-7 in MultiByteToWideChar. (try 2)

[Alexandre Julliard]

Alex Henrie  writes:
> Alexandre stated that he wanted to see more tests for invalid input:
> https://www.winehq.org/pipermail/wine-devel/2012-December/098051.html
>
> The new patchset includes many more tests for invalid input, and
> writing those tests exposed a bug in utf7_mbstowcs which is now fixed.
> The 14 examples in test_utf7_string_conversion add 29 tests which go
> through literally hundreds of code paths.
>
> Alexandre: Please, communicate why you are rejecting the code. This
> issue will drag on and on until we can come to an agreement.

You're still not checking the source len correctly, and you still don't
have any tests for such cases. And no, I don't consider that 14 examples
is a lot of tests. You should be able to do better than that.

[PATCH 1/3] kernel32: Support UTF-7 in MultiByteToWideChar. (try 2)

[Alex Henrie]

slackner, puk, and Andre_H gave me a ton of help over IRC over the
past couple of days (most of all slackner). The revised code is
sitting in https://github.com/alexhenrie/wine/commits/master so that
more people can review it, but I think it has a much better chance of
meeting Alexandre's standards now.

-Alex