Re: [PATCH] test-ctype: check EOF
- From
René Scharfe <l.s.r@web.de>
- Date
- May 1, 2023, 19:51 UTC
- Message-ID
- <1dbbd7ce-2b3f-6f88-f902-1d4ffe62e259@web.de>
- In-Reply-To
- <xmqqedo0849m.fsf@gitster.g>
Am 01.05.23 um 17:23 schrieb Junio C Hamano:
Show 41 quoted lines
> René Scharfe <l.s.r@web.de> writes:
>
>> The character classifiers are supposed to handle EOF, a negative integer
>> value. It isn't part of any character class. Extend the ctype tests to
>> cover it.
>
> The goal makes sense.
>
>> -static void report_error(const char *class, int ch)
>> +static void test(int ch, const char *class, int expect, int actual)
>> {
>> + if (actual == expect)
>> + return;
>> printf("%s classifies char %d (0x%02x) wrongly\n", class, ch, ch);
>> rc = 1;
>> }
>> @@ -24,9 +26,9 @@ static int is_in(const char *s, int ch)
>> #define TEST_CLASS(t,s) { \
>> int i; \
>> for (i = 0; i < 256; i++) { \
>> - if (is_in(s, i) != t(i)) \
>> - report_error(#t, i); \
>> + test(i, #t, is_in(s, i), t(i)); \
>> } \
>> + test(EOF, #t, 0, t(EOF)); \
>> }
>>
>> #define DIGIT "0123456789"
>> --
>> 2.40.1
>
> I am a bit torn on the conversion from report_error() to test(), as
> the only "test"-y thing the updated function does is to compare two
> of its parameters. It still is mostly about reporting an error when
> something goes wrong. In other words, the added change could have
> been just
>
> if (t(EOF) != 0)
> report_error(#t, EOF);
>
> after the loop, I think.True -- the objective can be met without changing the function, and whatever the goal for the "while at it" refactoring was can be discussed in separate patch, if necessary.
Show 6 quoted lines
> The only thing that I am not entirely happy with the end result is > the name of the function, and not how the caller and the callee > divides their work, so it is so miniscule a thing that it won't be > worth our collective time to bikeshed it further. > > Let's take it as-is. Thanks.
Hmm, okay. Here's v2 anyway; feel free to ignore it. At least I found this reminder to stay focused and KISS helpful.
--- >8 --- Subject: [PATCH v2] test-ctype: check EOF
The character classifiers are supposed to allow passing EOF to them, a negative value. It isn't part of any character class. Extend the tests to cover that.
Signed-off-by: René Scharfe <l.s.r@web.de> --- t/helper/test-ctype.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/t/helper/test-ctype.c b/t/helper/test-ctype.c index 71a1a5c9b0..e5659df40b 100644 --- a/t/helper/test-ctype.c +++ b/t/helper/test-ctype.c @@ -27,6 +27,8 @@ static int is_in(const char *s, int ch) if (is_in(s, i) != t(i)) \ report_error(#t, i); \ } \ + if (t(EOF)) \ + report_error(#t, EOF); \ } #define DIGIT "0123456789" -- 2.40.1