[PATCH v2 2/3] wcsmbs: Add test-wcsstr

[Adhemerval Zanella Netto]

On 28/02/24 21:45, DJ Delorie wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>> Parametrize test-strstr.c so it can be used to check wcsstr.
> 
> LGTM although I suggest renaming one of the variables to more accurately
> reflect its new role.  Also a question about error() formatting, but you
> could commit this as-is if you want.
> 
> Reviewed-by: DJ Delorie <dj@redhat.com>
> 
>> diff --git a/string/test-strstr.c b/string/test-strstr.c
>> index 05d0b7c98c..f82aeb2cfa 100644
>> --- a/string/test-strstr.c
>> +++ b/string/test-strstr.c
>> @@ -17,16 +17,44 @@
>>     <https://www.gnu.org/licenses/>.  */
>>  
>>  #define TEST_MAIN
>> -#define TEST_NAME "strstr"
>> -#include "test-string.h"
>> +#ifndef WIDE
>> +# define TEST_NAME "strstr"
>> +#else
>> +# define TEST_NAME "wcsstr"
>> +#endif
> 
> Ok.
> 
>> +#ifndef WIDE
>> +# define CHAR char
>> +# define STRLEN strlen
>> +# define STRSTR strstr
>> +# 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 STRSTR wcsstr
>> +# 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
>> +   (considering 4kb page size).  */
>> +# define BUF1PAGES 4
>> +#endif
>>  
>> +#include "test-string.h"
> 
> 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);
>>  
>>    if (s2len > s1len)
>>      return NULL;
>> @@ -38,28 +66,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;
>>      }
>>  
>>    return NULL;
>>  }
> 
> Ok.
> 
> 
>> -typedef char *(*proto_t) (const char *, const char *);
>> +typedef CHAR *(*proto_t) (const CHAR *, const CHAR *);
>>  
>> -IMPL (strstr, 1)
>> +IMPL (STRSTR, 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)
> 
> Ok.
> 
>>  {
>> -  char *result = CALL (impl, s1, s2);
>> +  CHAR *result = CALL (impl, s1, s2);
> 
> Ok.
> 
>>    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 %p %p %p", impl->name,
>> +	     result, exp_result);
> 
> Ok... but do we not have a way to printf() wide character strings?

Indeed, I think we can use "%Ls" here. I think I have used %p just to avoid
adding another parametrized macro.  I will fix it.

> 
>> @@ -68,7 +95,7 @@ check_result (impl_t *impl, const char *s1, const char *s2,
>>  }
>>  
>>  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.
> 
>> @@ -79,49 +106,51 @@ static void
>>  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);
>> -  char *ss2 = s2;
>> +  CHAR *s1 = (CHAR *) (buf1 + align1);
>> +  CHAR *s2 = (CHAR *) (buf2 + align2);
> 
> Ok.
> 
>> -  static const char d[] = "1234567890abcdef";
>> -#define dl (sizeof (d) - 1)
>> +  static const CHAR d[] = L("1234567890abcdef");
>> +  const size_t dl = STRLEN (d);
> 
> Ok.  The choice of L() macro confused me at first (I read L"" instead of
> L("")) but it works.
> 
>> +  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);
> 
> Ok.
> 
>>    s2[len2] = '\0';
>>  
>>    if (fail)
>>      {
>> -      char *ss1 = s1;
>> +      CHAR *ss1 = s1;
> 
> Ok.
> 
>>        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);
> 
> 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.
> 
>>  static void
>>  check2 (void)
>>  {
>> -  const char s1_stack[] = ", enable_static, \0, enable_shared, ";
>> +  const CHAR s1_stack[] = L(", enable_static, \0, enable_shared, ");
> 
> Ok.
> 
>>    const size_t s1_byte_count = 18;
> 
> I think this needs to be renamed to s1_char_count or something, since
> with wide characters, S1 is no longer 18 bytes long.

Ack.

> 
>> -  const char *s2_stack = &(s1_stack[s1_byte_count]);
>> -  const size_t s2_byte_count = 18;
>> -  char *exp_result;
>> +  const size_t s1_byte_len = 18 * sizeof (CHAR);
>> +  const CHAR *s2_stack = &(s1_stack[s1_byte_count]);
>> +  const size_t s2_byte_len = 18 * sizeof (CHAR);;
>> +  CHAR *exp_result;
> 
> 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.
> 
>>  static void
>>  pr23637 (void)
>>  {
>> -  char *h = (char*) buf1;
>> -  char *n = (char*) buf2;
>> +  CHAR *h = (CHAR*) buf1;
>> +  CHAR *n = (CHAR*) buf2;
> 
> Ok.
> 
>> -  char *exp_result = simple_strstr (h, n);
>> +  CHAR *exp_result = simple_strstr (h, n);
> 
> Ok.
> 
>> diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile
>> +  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.
>