On 03/14/13 18:49, Markus Armbruster wrote:
> These are all broken, too.
What are "these"? And how are they broken? And how does the patch fix them?
>
> A few test cases use noncharacters U+FFFF and U+10FFFF. Risks testing
> noncharacters some more instead of what they're supposed to test. Use
> U+FFFD and U+10FFFD instead.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
> tests/check-qjson.c | 85
> +++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 72 insertions(+), 13 deletions(-)
I'm confused about the commit message. There are three paragraphs in it
(the title, the first paragraph, and the 2nd paragraph). This patch
modifies different tests:
> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
> index 852124a..efec1b2 100644
> --- a/tests/check-qjson.c
> +++ b/tests/check-qjson.c
> @@ -158,7 +158,7 @@ static void utf8_string(void)
> * consider using overlong encoding \xC0\x80 for U+0000 ("modified
> * UTF-8").
> *
> - * Test cases are scraped from Markus Kuhn's UTF-8 decoder
> + * Most test cases are scraped from Markus Kuhn's UTF-8 decoder
> * capability and stress test at
> * http://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt
> */
> @@ -256,11 +256,11 @@ static void utf8_string(void)
> "\xDF\xBF",
> "\"\\u07FF\"",
> },
> - /* 2.2.3 3 bytes U+FFFF */
> + /* 2.2.3 3 bytes U+FFFD */
> {
> - "\"\xEF\xBF\xBF\"",
> - "\xEF\xBF\xBF",
> - "\"\\uFFFF\"",
> + "\"\xEF\xBF\xBD\"",
> + "\xEF\xBF\xBD",
> + "\"\\uFFFD\"",
> },
This is under "2.2 Last possible sequence of a certain length". I guess
this is where you say "last possible sequence of a certain length,
encoding a character (= non-noncharacter)". OK, p#2.
> /* 2.2.4 4 bytes U+1FFFFF */
> {
> @@ -303,10 +303,10 @@ static void utf8_string(void)
> "\"\\uFFFD\"",
> },
> {
> - /* U+10FFFF */
> - "\"\xF4\x8F\xBF\xBF\"",
> - "\xF4\x8F\xBF\xBF",
> - "\"\\u43FF\\uFFFF\"", /* bug: want "\"\\uDBFF\\uDFFF\"" */
> + /* U+10FFFD */
> + "\"\xF4\x8F\xBF\xBD\"",
> + "\xF4\x8F\xBF\xBD",
> + "\"\\u43FF\\uFFFF\"", /* bug: want "\"\\uDBFF\\uDFFD\"" */
> },
> {
> /* U+110000 */
Under "2.3 Other boundary conditions". Not a non-character any longer,
but also not a boundary condition. At least not the original one. Still
covered by the ...FFFD part of the commit message, p#2.
> @@ -584,9 +584,9 @@ static void utf8_string(void)
> "\"\\u07FF\"",
> },
> {
> - /* \U+FFFF */
> - "\"\xF0\x8F\xBF\xBF\"",
> - "\xF0\x8F\xBF\xBF", /* bug: not corrected */
> + /* \U+FFFD */
> + "\"\xF0\x8F\xBF\xBD\"",
> + "\xF0\x8F\xBF\xBD", /* bug: not corrected */
> "\"\\u03FF\\uFFFF\"", /* bug: want "\"\\uFFFF\"" */
> },
> {
Under "4.2 Maximum overlong sequences". What does that even mean? "In
some sense maximum codepoints, all represented as overlong sequences"? P#2.
> @@ -731,6 +731,7 @@ static void utf8_string(void)
> "\"\\uDBFF\\uDFFF\"", /* bug: want "\"\\uFFFF\\uFFFF\"" */
> },
> /* 5.3 Other illegal code positions */
> + /* BMP noncharacters */
> {
> /* \U+FFFE */
> "\"\xEF\xBF\xBE\"",
> @@ -741,7 +742,65 @@ static void utf8_string(void)
> /* \U+FFFF */
> "\"\xEF\xBF\xBF\"",
> "\xEF\xBF\xBF", /* bug: not corrected */
> - "\"\\uFFFF\"", /* bug: not corrected */
> + "\"\\uFFFF\"",
> + },
> + {
> + /* U+FDD0 */
> + "\"\xEF\xB7\x90\"",
> + "\xEF\xB7\x90", /* bug: not corrected */
> + "\"\\uFDD0\"", /* bug: not corrected */
> + },
> + {
> + /* U+FDEF */
> + "\"\xEF\xB7\xAF\"",
> + "\xEF\xB7\xAF", /* bug: not corrected */
> + "\"\\uFDEF\"", /* bug: not corrected */
> + },
> + /* Plane 1 .. 16 noncharacters */
> + {
> + /* U+1FFFE U+1FFFF U+2FFFE U+2FFFF ... U+10FFFE U+10FFFF */
> + "\"\xF0\x9F\xBF\xBE\xF0\x9F\xBF\xBF"
> + "\xF0\xAF\xBF\xBE\xF0\xAF\xBF\xBF"
> + "\xF0\xBF\xBF\xBE\xF0\xBF\xBF\xBF"
> + "\xF1\x8F\xBF\xBE\xF1\x8F\xBF\xBF"
> + "\xF1\x9F\xBF\xBE\xF1\x9F\xBF\xBF"
> + "\xF1\xAF\xBF\xBE\xF1\xAF\xBF\xBF"
> + "\xF1\xBF\xBF\xBE\xF1\xBF\xBF\xBF"
> + "\xF2\x8F\xBF\xBE\xF2\x8F\xBF\xBF"
> + "\xF2\x9F\xBF\xBE\xF2\x9F\xBF\xBF"
> + "\xF2\xAF\xBF\xBE\xF2\xAF\xBF\xBF"
> + "\xF2\xBF\xBF\xBE\xF2\xBF\xBF\xBF"
> + "\xF3\x8F\xBF\xBE\xF3\x8F\xBF\xBF"
> + "\xF3\x9F\xBF\xBE\xF3\x9F\xBF\xBF"
> + "\xF3\xAF\xBF\xBE\xF3\xAF\xBF\xBF"
> + "\xF3\xBF\xBF\xBE\xF3\xBF\xBF\xBF"
> + "\xF4\x8F\xBF\xBE\xF4\x8F\xBF\xBF\"",
> + /* bug: not corrected */
> + "\xF0\x9F\xBF\xBE\xF0\x9F\xBF\xBF"
> + "\xF0\xAF\xBF\xBE\xF0\xAF\xBF\xBF"
> + "\xF0\xBF\xBF\xBE\xF0\xBF\xBF\xBF"
> + "\xF1\x8F\xBF\xBE\xF1\x8F\xBF\xBF"
> + "\xF1\x9F\xBF\xBE\xF1\x9F\xBF\xBF"
> + "\xF1\xAF\xBF\xBE\xF1\xAF\xBF\xBF"
> + "\xF1\xBF\xBF\xBE\xF1\xBF\xBF\xBF"
> + "\xF2\x8F\xBF\xBE\xF2\x8F\xBF\xBF"
> + "\xF2\x9F\xBF\xBE\xF2\x9F\xBF\xBF"
> + "\xF2\xAF\xBF\xBE\xF2\xAF\xBF\xBF"
> + "\xF2\xBF\xBF\xBE\xF2\xBF\xBF\xBF"
> + "\xF3\x8F\xBF\xBE\xF3\x8F\xBF\xBF"
> + "\xF3\x9F\xBF\xBE\xF3\x9F\xBF\xBF"
> + "\xF3\xAF\xBF\xBE\xF3\xAF\xBF\xBF"
> + "\xF3\xBF\xBF\xBE\xF3\xBF\xBF\xBF"
> + "\xF4\x8F\xBF\xBE\xF4\x8F\xBF\xBF",
> + /* bug: not corrected */
> + "\"\\u07FF\\uFFFF\\u07FF\\uFFFF\\u0BFF\\uFFFF\\u0BFF\\uFFFF"
> + "\\u0FFF\\uFFFF\\u0FFF\\uFFFF\\u13FF\\uFFFF\\u13FF\\uFFFF"
> + "\\u17FF\\uFFFF\\u17FF\\uFFFF\\u1BFF\\uFFFF\\u1BFF\\uFFFF"
> + "\\u1FFF\\uFFFF\\u1FFF\\uFFFF\\u23FF\\uFFFF\\u23FF\\uFFFF"
> + "\\u27FF\\uFFFF\\u27FF\\uFFFF\\u2BFF\\uFFFF\\u2BFF\\uFFFF"
> + "\\u2FFF\\uFFFF\\u2FFF\\uFFFF\\u33FF\\uFFFF\\u33FF\\uFFFF"
> + "\\u37FF\\uFFFF\\u37FF\\uFFFF\\u3BFF\\uFFFF\\u3BFF\\uFFFF"
> + "\\u3FFF\\uFFFF\\u3FFF\\uFFFF\\u43FF\\uFFFF\\u43FF\\uFFFF\"",
> },
> {}
> };
>
This is probably p#0 (the title).
Ah. Have you removed the noncharacters from the other tests, but made up
for them at the end with new noncharacter tests?
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
