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

[Jeff King <peff@peff.net>]

On Sun, Feb 25, 2024 at 01:41:30PM -0500, Eric Sunshine wrote:

> On Sun, Feb 25, 2024 at 1:28 PM René Scharfe <l.s.r@web.de> wrote:
> > 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`?

That would defeat the advertised purpose that we can handle embedded
NULs, though. Whereas with sizeof(), I think a literal like "foo\0bar"
would still have length 8.

-Peff