Re: [PATCH v3 1/2] wcsmbs: Add test-wcsstr

[DJ Delorie <dj@redhat.com>]

One typo.  LGTM otherwise

Reviewed-by: DJ Delorie <dj@redhat.com>

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> diff --git a/string/test-strstr.c b/string/test-strstr.c
> index 4115f7d2fd..e628a3478a 100644
> --- a/string/test-strstr.c
> +++ b/string/test-strstr.c
> @@ -17,21 +17,56 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #define TEST_MAIN
> -#define TEST_NAME "strstr"
> -#include "test-string.h"
> +#ifndef WIDE
> +# define TEST_NAME "strstr"
> +# define TEST_FUNC strstr
> +#else
> +# define TEST_NAME "wcsstr"
> +# define TEST_FUNC wcsstr
> +#endif

Ok.

> +#ifndef WIDE
> +# define CHAR char
> +# define STRLEN strlen
> +# define STRCPY strcpy
> +# define MEMCPY memcpy
> +# define MEMSET memset
> +# define MEMPCPY mempcpy
> +# define L(s) s
> +#else
> +# include <wchar.h>
> +# define CHAR wchar_t
> +# define STRLEN wcslen
> +# define STRCPY wcscpy
> +# define MEMCPY wmemcpy
> +# define MEMSET wmemset
> +# define MEMPCPY wmempcpy
> +# define L(s) L ## s
> +/* The test requires up to 8191 charateres, so allocate at least 32Kb

Spelling: "characters"

> +   (considering 4kb page size).  */
> +# define BUF1PAGES 4
> +#endif

Ok.

> +#include "test-string.h"

Ok.

> -#define STRSTR c_strstr
> -#define libc_hidden_builtin_def(arg) /* nothing */
> -#define __strnlen strnlen
> -#include "strstr.c"
> +#ifndef WIDE
> +# define STRSTR c_strstr
> +# define libc_hidden_builtin_def(arg) /* nothing */
> +# define __strnlen strnlen
> +# include "strstr.c"
> +# define C_IMPL STRSTR
> +#else
> +# define WCSSTR c_wcsstr
> +# include "wcsstr.c"
> +# define C_IMPL WCSSTR
> +#endif

Ok.

>  /* Naive implementation to verify results.  */
> -static char *
> -simple_strstr (const char *s1, const char *s2)
> +static CHAR *
> +simple_strstr (const CHAR *s1, const CHAR *s2)
>  {
> -  ssize_t s1len = strlen (s1);
> -  ssize_t s2len = strlen (s2);
> +  ssize_t s1len = STRLEN (s1);
> +  ssize_t s2len = STRLEN (s2);

Ok.

> @@ -43,29 +78,27 @@ simple_strstr (const char *s1, const char *s2)
>  	if (s1[i + j] != s2[j])
>  	  break;
>        if (j == s2len)
> -	return (char *) s1 + i;
> +	return (CHAR *) s1 + i;
>      }

Ok.

> -typedef char *(*proto_t) (const char *, const char *);
> -
> -IMPL (c_strstr, 0)
> -IMPL (strstr, 1)
> +typedef CHAR *(*proto_t) (const CHAR *, const CHAR *);
>  
> +IMPL (C_IMPL, 1)
> +IMPL (TEST_FUNC, 1)

Ok.

>  static int
> -check_result (impl_t *impl, const char *s1, const char *s2,
> -	      char *exp_result)
> +check_result (impl_t *impl, const CHAR *s1, const CHAR *s2,
> +	      CHAR *exp_result)
>  {
> -  char *result = CALL (impl, s1, s2);
> +  CHAR *result = CALL (impl, s1, s2);
>    if (result != exp_result)
>      {
> -      error (0, 0, "Wrong result in function %s %s %s", impl->name,
> -	     (result == NULL) ? "(null)" : result,
> -	     (exp_result == NULL) ? "(null)" : exp_result);
> +      error (0, 0, "Wrong result in function %s %p %p", impl->name,
> +	     result, exp_result);
>        ret = 1;
>        return -1;
>      }

Ok.

>  static void
> -do_one_test (impl_t *impl, const char *s1, const char *s2, char *exp_result)
> +do_one_test (impl_t *impl, const CHAR *s1, const CHAR *s2, CHAR *exp_result)

Ok.

>  do_test (size_t align1, size_t align2, size_t len1, size_t len2,
>  	 int fail)
>  {
> -  char *s1 = (char *) (buf1 + align1);
> -  char *s2 = (char *) (buf2 + align2);
> +  align1 = align1 * sizeof (CHAR);
> +  align2 = align2 * sizeof (CHAR);

Ok.

> -  static const char d[] = "1234567890abcdef";
> -#define dl (sizeof (d) - 1)
> -  char *ss2 = s2;
> +  CHAR *s1 = (CHAR *) (buf1 + align1);
> +  CHAR *s2 = (CHAR *) (buf2 + align2);
> +
> +  static const CHAR d[] = L("1234567890abcdef");
> +  const size_t dl = STRLEN (d);
> +  CHAR *ss2 = s2;

Ok.

>    for (size_t l = len2; l > 0; l = l > dl ? l - dl : 0)
>      {
>        size_t t = l > dl ? dl : l;
> -      ss2 = mempcpy (ss2, d, t);
> +      ss2 = MEMPCPY (ss2, d, t);
>      }
>    s2[len2] = '\0';

Ok.

>    if (fail)
>      {
> -      char *ss1 = s1;
> +      CHAR *ss1 = s1;
>        for (size_t l = len1; l > 0; l = l > dl ? l - dl : 0)
>  	{
>  	  size_t t = l > dl ? dl : l;
> -	  memcpy (ss1, d, t);
> +	  MEMCPY (ss1, d, t);
>  	  ++ss1[len2 > 7 ? 7 : len2 - 1];
>  	  ss1 += t;

Ok.

>      {
> -      memset (s1, '0', len1);
> -      memcpy (s1 + len1 - len2, s2, len2);
> +      MEMSET (s1, '0', len1);
> +      MEMCPY (s1 + len1 - len2, s2, len2);

Ok.

>  static void
>  check1 (void)
>  {
> -  const char s1[] =
> -    "F_BD_CE_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_C3_88_20_EF_BF_BD_EF_BF_BD_EF_BF_BD_C3_A7_20_EF_BF_BD";
> -  const char s2[] = "_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD";
> -  char *exp_result;
> +  const CHAR s1[] =
> +    L("F_BD_CE_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_C3_88_20_EF_BF_BD_EF_BF_BD_EF_BF_BD_C3_A7_20_EF_BF_BD");
> +  const CHAR s2[] = L("_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD_EF_BF_BD");
> +  CHAR *exp_result;

Ok.

> @@ -137,30 +172,31 @@ check1 (void)
>  static void
>  check2 (void)
>  {
> -  const char s1_stack[] = ", enable_static, \0, enable_shared, ";
> -  const size_t s1_byte_count = 18;
> -  const char *s2_stack = &(s1_stack[s1_byte_count]);
> -  const size_t s2_byte_count = 18;
> -  char *exp_result;
> +  const CHAR s1_stack[] = L(", enable_static, \0, enable_shared, ");
> +  const size_t s1_char_count = 18;
> +  const size_t s1_byte_len = 18 * sizeof (CHAR);
> +  const CHAR *s2_stack = &(s1_stack[s1_char_count]);
> +  const size_t s2_byte_len = 18 * sizeof (CHAR);
> +  CHAR *exp_result;
>    const size_t page_size_real = getpagesize ();

Ok.

>    /* Haystack at end of page.  The following page is protected.  */
> -  char *s1_page_end = (void *) buf1 + page_size - s1_byte_count;
> -  strcpy (s1_page_end, s1_stack);
> +  CHAR *s1_page_end = (void *) buf1 + page_size - s1_byte_len;
> +  STRCPY (s1_page_end, s1_stack);

Ok.

>    /* Haystack which crosses a page boundary.
>       Note: page_size is at least 2 * getpagesize.  See test_init.  */
> -  char *s1_page_cross = (void *) buf1 + page_size_real - 8;
> -  strcpy (s1_page_cross, s1_stack);
> +  CHAR *s1_page_cross = (void *) buf1 + page_size_real - 8;
> +  STRCPY (s1_page_cross, s1_stack);

Ok.

>    /* Needle at end of page.  The following page is protected.  */
> -  char *s2_page_end = (void *) buf2 + page_size - s2_byte_count;
> -  strcpy (s2_page_end, s2_stack);
> +  CHAR *s2_page_end = (void *) buf2 + page_size - s2_byte_len;
> +  STRCPY (s2_page_end, s2_stack);

Ok.

>    /* Needle which crosses a page boundary.
>       Note: page_size is at least 2 * getpagesize.  See test_init.  */
> -  char *s2_page_cross = (void *) buf2 + page_size_real - 8;
> -  strcpy (s2_page_cross, s2_stack);
> +  CHAR *s2_page_cross = (void *) buf2 + page_size_real - 8;
> +  STRCPY (s2_page_cross, s2_stack);

Ok.

> @@ -184,8 +220,8 @@ check2 (void)
>  static void
>  pr23637 (void)
>  {
> -  char *h = (char*) buf1;
> -  char *n = (char*) buf2;
> +  CHAR *h = (CHAR*) buf1;
> +  CHAR *n = (CHAR*) buf2;

Ok.

> @@ -200,7 +236,7 @@ pr23637 (void)
>    /* Ensure we don't match at the first 'x'.  */
>    h[0] = 'x';
>  
> -  char *exp_result = simple_strstr (h, n);
> +  CHAR *exp_result = simple_strstr (h, n);

Ok.

> diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile
>    test-wcspbrk \
>    test-wcsrchr \
>    test-wcsspn \
> +  test-wcsstr \

Ok.

> diff --git a/wcsmbs/test-wcsstr.c b/wcsmbs/test-wcsstr.c
> +/* Test wcsstr function.
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#define WIDE 1
> +#include <string/test-strstr.c>

Ok.

> diff --git a/wcsmbs/wcsstr.c b/wcsmbs/wcsstr.c
> index 78f1cc9ce0..ec5687e8d7 100644
> --- a/wcsmbs/wcsstr.c
> +++ b/wcsmbs/wcsstr.c
> @@ -28,8 +28,12 @@
>  
>  #include <wchar.h>
>  
> +#ifndef WCSSTR
> +# define WCSSTR wcsstr
> +#endif
> +

Ok.

>  wchar_t *
> -wcsstr (const wchar_t *haystack, const wchar_t *needle)
> +WCSSTR (const wchar_t *haystack, const wchar_t *needle)

Ok.
> @@ -92,6 +96,8 @@ foundneedle:
>  ret0:
>    return NULL;
>  }
> +#ifndef WCSSTR
>  /* This alias is for backward compatibility with drafts of the ISO C
>     standard.  Unfortunately the Unix(TM) standard requires this name.  */
>  weak_alias (wcsstr, wcswcs)
> +#endif

Ok.