{"thread":{"id":"59673","subject":"[PATCH] test-ctype: check EOF","startedAt":"2023-04-30T10:00:14Z","lastAt":"2023-05-01T19:52:07Z","messageCount":3,"participants":["René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"476317","messageId":"cbc7feb9-7e15-77e2-b3e9-1fa194b4d4fb@web.de","threadId":"59673","inReplyTo":null,"subject":"[PATCH] test-ctype: check EOF","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2023-04-30T10:00:01Z","receivedAt":"2023-04-30T10:00:14Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"The character classifiers are supposed to handle EOF, a negative integer\nvalue.  It isn't part of any character class.  Extend the ctype tests to\ncover it.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n t/helper/test-ctype.c | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/t/helper/test-ctype.c b/t/helper/test-ctype.c\nindex 71a1a5c9b0..039703ee72 100644\n--- a/t/helper/test-ctype.c\n+++ b/t/helper/test-ctype.c\n@@ -2,8 +2,10 @@\n\n static int rc;\n\n-static void report_error(const char *class, int ch)\n+static void test(int ch, const char *class, int expect, int actual)\n {\n+\tif (actual == expect)\n+\t\treturn;\n \tprintf(\"%s classifies char %d (0x%02x) wrongly\\n\", class, ch, ch);\n \trc = 1;\n }\n@@ -24,9 +26,9 @@ static int is_in(const char *s, int ch)\n #define TEST_CLASS(t,s) {\t\t\t\\\n \tint i;\t\t\t\t\t\\\n \tfor (i = 0; i < 256; i++) {\t\t\\\n-\t\tif (is_in(s, i) != t(i))\t\\\n-\t\t\treport_error(#t, i);\t\\\n+\t\ttest(i, #t, is_in(s, i), t(i));\t\\\n \t}\t\t\t\t\t\\\n+\ttest(EOF, #t, 0, t(EOF));\t\t\\\n }\n\n #define DIGIT \"0123456789\"\n--\n2.40.1\n"},{"id":"476336","messageId":"xmqqedo0849m.fsf@gitster.g","threadId":"59673","inReplyTo":"cbc7feb9-7e15-77e2-b3e9-1fa194b4d4fb@web.de","subject":"Re: [PATCH] test-ctype: check EOF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-01T15:23:17Z","receivedAt":"2023-05-01T15:23:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> The character classifiers are supposed to handle EOF, a negative integer\n> value.  It isn't part of any character class.  Extend the ctype tests to\n> cover it.\n\nThe goal makes sense.\n\n> -static void report_error(const char *class, int ch)\n> +static void test(int ch, const char *class, int expect, int actual)\n>  {\n> +\tif (actual == expect)\n> +\t\treturn;\n>  \tprintf(\"%s classifies char %d (0x%02x) wrongly\\n\", class, ch, ch);\n>  \trc = 1;\n>  }\n> @@ -24,9 +26,9 @@ static int is_in(const char *s, int ch)\n>  #define TEST_CLASS(t,s) {\t\t\t\\\n>  \tint i;\t\t\t\t\t\\\n>  \tfor (i = 0; i < 256; i++) {\t\t\\\n> -\t\tif (is_in(s, i) != t(i))\t\\\n> -\t\t\treport_error(#t, i);\t\\\n> +\t\ttest(i, #t, is_in(s, i), t(i));\t\\\n>  \t}\t\t\t\t\t\\\n> +\ttest(EOF, #t, 0, t(EOF));\t\t\\\n>  }\n>\n>  #define DIGIT \"0123456789\"\n> --\n> 2.40.1\n\nI am a bit torn on the conversion from report_error() to test(), as\nthe only \"test\"-y thing the updated function does is to compare two\nof its parameters.  It still is mostly about reporting an error when\nsomething goes wrong.  In other words, the added change could have\nbeen just\n\n\tif (t(EOF) != 0)\n\t\treport_error(#t, EOF);\n\nafter the loop, I think.\n\nThe only thing that I am not entirely happy with the end result is\nthe name of the function, and not how the caller and the callee\ndivides their work, so it is so miniscule a thing that it won't be\nworth our collective time to bikeshed it further.\n\nLet's take it as-is.  Thanks.\n"},{"id":"476375","messageId":"1dbbd7ce-2b3f-6f88-f902-1d4ffe62e259@web.de","threadId":"59673","inReplyTo":"xmqqedo0849m.fsf@gitster.g","subject":"Re: [PATCH] test-ctype: check EOF","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2023-05-01T19:51:57Z","receivedAt":"2023-05-01T19:52:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 01.05.23 um 17:23 schrieb Junio C Hamano:\n> René Scharfe <l.s.r@web.de> writes:\n>\n>> The character classifiers are supposed to handle EOF, a negative integer\n>> value.  It isn't part of any character class.  Extend the ctype tests to\n>> cover it.\n>\n> The goal makes sense.\n>\n>> -static void report_error(const char *class, int ch)\n>> +static void test(int ch, const char *class, int expect, int actual)\n>>  {\n>> +\tif (actual == expect)\n>> +\t\treturn;\n>>  \tprintf(\"%s classifies char %d (0x%02x) wrongly\\n\", class, ch, ch);\n>>  \trc = 1;\n>>  }\n>> @@ -24,9 +26,9 @@ static int is_in(const char *s, int ch)\n>>  #define TEST_CLASS(t,s) {\t\t\t\\\n>>  \tint i;\t\t\t\t\t\\\n>>  \tfor (i = 0; i < 256; i++) {\t\t\\\n>> -\t\tif (is_in(s, i) != t(i))\t\\\n>> -\t\t\treport_error(#t, i);\t\\\n>> +\t\ttest(i, #t, is_in(s, i), t(i));\t\\\n>>  \t}\t\t\t\t\t\\\n>> +\ttest(EOF, #t, 0, t(EOF));\t\t\\\n>>  }\n>>\n>>  #define DIGIT \"0123456789\"\n>> --\n>> 2.40.1\n>\n> I am a bit torn on the conversion from report_error() to test(), as\n> the only \"test\"-y thing the updated function does is to compare two\n> of its parameters.  It still is mostly about reporting an error when\n> something goes wrong.  In other words, the added change could have\n> been just\n>\n> \tif (t(EOF) != 0)\n> \t\treport_error(#t, EOF);\n>\n> after the loop, I think.\n\nTrue -- the objective can be met without changing the function, and\nwhatever the goal for the \"while at it\" refactoring was can be\ndiscussed in separate patch, if necessary.\n\n> The only thing that I am not entirely happy with the end result is\n> the name of the function, and not how the caller and the callee\n> divides their work, so it is so miniscule a thing that it won't be\n> worth our collective time to bikeshed it further.\n>\n> Let's take it as-is.  Thanks.\n\nHmm, okay.  Here's v2 anyway; feel free to ignore it.  At least I\nfound this reminder to stay focused and KISS helpful.\n\n--- >8 ---\nSubject: [PATCH v2] test-ctype: check EOF\n\nThe character classifiers are supposed to allow passing EOF to them, a\nnegative value.  It isn't part of any character class.  Extend the tests\nto cover that.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n t/helper/test-ctype.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/helper/test-ctype.c b/t/helper/test-ctype.c\nindex 71a1a5c9b0..e5659df40b 100644\n--- a/t/helper/test-ctype.c\n+++ b/t/helper/test-ctype.c\n@@ -27,6 +27,8 @@ static int is_in(const char *s, int ch)\n \t\tif (is_in(s, i) != t(i))\t\\\n \t\t\treport_error(#t, i);\t\\\n \t}\t\t\t\t\t\\\n+\tif (t(EOF))\t\t\t\t\\\n+\t\treport_error(#t, EOF);\t\t\\\n }\n\n #define DIGIT \"0123456789\"\n--\n2.40.1\n"}]}