Re: [PATCH 1/3] kernel32: Support UTF-7 in MultiByteToWideChar. (try 2)
Alex Henrie <alexhenrie24(a)gmail.com> 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. -- Alexandre Julliard julliard(a)winehq.org
2014-10-07 7:42 GMT-06:00 Alexandre Julliard <julliard(a)winehq.org>:
Alex Henrie <alexhenrie24(a)gmail.com> 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
On 10/07/2014 06:06 PM, Alex Henrie wrote:
2014-10-07 7:42 GMT-06:00 Alexandre Julliard <julliard(a)winehq.org>:
Alex Henrie <alexhenrie24(a)gmail.com> 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
2014-10-07 13:35 GMT-06:00 Michael Stefaniuc <mstefani(a)redhat.com>:
On 10/07/2014 06:06 PM, Alex Henrie wrote:
2014-10-07 7:42 GMT-06:00 Alexandre Julliard <julliard(a)winehq.org>:
Alex Henrie <alexhenrie24(a)gmail.com> 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
Alex Henrie <alexhenrie24(a)gmail.com> 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. -- Alexandre Julliard julliard(a)winehq.org
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
participants (3)
-
Alex Henrie -
Alexandre Julliard -
Michael Stefaniuc