Re: [PATCH 1/3] t-ctype: allow NUL anywhere in the specification string

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 25, 2024, 18:41 UTC
Message-ID: <CAPig+cThAYjf4DU40dY9jCnjL0FsVpYFhcsj9YdvW2Q=c=ZDBg@mail.gmail.com>
In-Reply-To: <5dea0535-4b21-483a-a10a-bfa8e9e85650@web.de>

On Sun, Feb 25, 2024 at 1:28 PM René Scharfe <l.s.r@web.de> wrote:

Show 22 quoted lines

> Am 25.02.24 um 19:05 schrieb Eric Sunshine:
> > On Sun, Feb 25, 2024 at 6:27 AM René Scharfe <l.s.r@web.de> wrote:
> >> Getting the string size using sizeof only works in a macro and with a
> >> string constant, but that's exactly what we have and I don't see it
> >> changing anytime soon.
> >>
> >>  /* Macro to test a character type */
> >>  #define TEST_CTYPE_FUNC(func, string) \
> >
> > Taking into consideration the commit message warning about string
> > constants, would it make sense to update the comment to mention that
> > limitation?
> >
> >     /* Test a character type. (Only use with string constants.) */
> >     #define TEST_CTYPE_FUNC(func, string) \
> >>  static void test_ctype_##func(void) { \
> >>         for (int i = 0; i < 256; i++) { \
> >> +               int expect = !!memchr(string, i, sizeof(string) - 1); \
>
> I think the temptation to pass a string pointer is low -- if only
> because there aren't any in this file.  But adding such a warning
> can't hurt, so yeah, why not?

The patch just posted[1] by SZEDER reminded me that, on this project, we assume that the compiler is smart enough to replace `strlen("string-literal")` with the constant `15`, so rather than worrying about updating comment to mention the sizeof() limitation, you could perhaps just use `strlen(string)` instead of `sizeof(string)-1`?

[1]: https://lore.kernel.org/git/20240225183452.1939334-1-szeder.dev@gmail.com/