git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [PATCH] test-ctype: check EOF
Date: Mon, 01 May 2023 08:23:17 -0700	[thread overview]
Message-ID: <xmqqedo0849m.fsf@gitster.g> (raw)
In-Reply-To: <cbc7feb9-7e15-77e2-b3e9-1fa194b4d4fb@web.de> ("René Scharfe"'s message of "Sun, 30 Apr 2023 12:00:01 +0200")

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.

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.

  reply	other threads:[~2023-05-01 15:23 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-30 10:00 [PATCH] test-ctype: check EOF René Scharfe
2023-05-01 15:23 ` Junio C Hamano [this message]
2023-05-01 19:51   ` René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqedo0849m.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).