diff mbox

[v2,12/12] posix: Remove VLA usage for internal fnmatch implementation

Message ID 1517837254-19399-13-git-send-email-adhemerval.zanella@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella Netto Feb. 5, 2018, 1:27 p.m. UTC 
  Checked on x86_64-linux-gnu.

	* posix/fnmatch.c: Include char_array required files.
	* posix/fnmatch_loop.c (FCT): Replace VLA with char_array.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog            |  3 +++
 posix/fnmatch.c      |  1 +
 posix/fnmatch_loop.c | 68 +++++++++++++++++++++++++---------------------------
 3 files changed, 36 insertions(+), 36 deletions(-)

Comments

Andreas Schwab Feb. 5, 2018, 1:40 p.m. UTC | #1 
On Feb 05 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
> index eadb343..831e5ba 100644
> --- a/posix/fnmatch_loop.c
> +++ b/posix/fnmatch_loop.c
> @@ -493,26 +493,20 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
>  			  {
>  			    int32_t table_size;
>  			    const int32_t *symb_table;
> -# if WIDE_CHAR_VERSION
> -			    char str[c1];
> -			    unsigned int strcnt;
> -# else
> -#  define str (startp + 1)
> -# endif
> +			    struct char_array str;
> +			    char_array_init_empty (&str);
>  			    const unsigned char *extra;
>  			    int32_t idx;
>  			    int32_t elem;
>  			    int32_t second;
>  			    int32_t hash;
>  
> -# if WIDE_CHAR_VERSION
>  			    /* We have to convert the name to a single-byte
>  			       string.  This is possible since the names
>  			       consist of ASCII characters and the internal
>  			       representation is UCS4.  */
> -			    for (strcnt = 0; strcnt < c1; ++strcnt)
> -			      str[strcnt] = startp[1 + strcnt];
> -#endif
> +			    for (size_t strcnt = 0; strcnt < c1; ++strcnt)
> +			      char_array_append_char (&str, startp[1 + strcnt]);
>  
>  			    table_size =
>  			      _NL_CURRENT_WORD (LC_COLLATE,

That needs to be removed altogether, see
<http://sourceware.org/ml/libc-alpha/2017-04/msg00068.html>.

Andreas.
Adhemerval Zanella Netto Feb. 5, 2018, 3:08 p.m. UTC | #2 
On 05/02/2018 11:40, Andreas Schwab wrote:
> On Feb 05 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
>> index eadb343..831e5ba 100644
>> --- a/posix/fnmatch_loop.c
>> +++ b/posix/fnmatch_loop.c
>> @@ -493,26 +493,20 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
>>  			  {
>>  			    int32_t table_size;
>>  			    const int32_t *symb_table;
>> -# if WIDE_CHAR_VERSION
>> -			    char str[c1];
>> -			    unsigned int strcnt;
>> -# else
>> -#  define str (startp + 1)
>> -# endif
>> +			    struct char_array str;
>> +			    char_array_init_empty (&str);
>>  			    const unsigned char *extra;
>>  			    int32_t idx;
>>  			    int32_t elem;
>>  			    int32_t second;
>>  			    int32_t hash;
>>  
>> -# if WIDE_CHAR_VERSION
>>  			    /* We have to convert the name to a single-byte
>>  			       string.  This is possible since the names
>>  			       consist of ASCII characters and the internal
>>  			       representation is UCS4.  */
>> -			    for (strcnt = 0; strcnt < c1; ++strcnt)
>> -			      str[strcnt] = startp[1 + strcnt];
>> -#endif
>> +			    for (size_t strcnt = 0; strcnt < c1; ++strcnt)
>> +			      char_array_append_char (&str, startp[1 + strcnt]);
>>  
>>  			    table_size =
>>  			      _NL_CURRENT_WORD (LC_COLLATE,
> 
> That needs to be removed altogether, see
> <http://sourceware.org/ml/libc-alpha/2017-04/msg00068.html>.

Nice, I missed your patch.  It seems to apply clean, I will review on the original
thread.
Adhemerval Zanella Netto Feb. 5, 2018, 8:20 p.m. UTC | #3 
On 05/02/2018 13:08, Adhemerval Zanella wrote:
> 
> 
> On 05/02/2018 11:40, Andreas Schwab wrote:
>> On Feb 05 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>>
>>> diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
>>> index eadb343..831e5ba 100644
>>> --- a/posix/fnmatch_loop.c
>>> +++ b/posix/fnmatch_loop.c
>>> @@ -493,26 +493,20 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
>>>  			  {
>>>  			    int32_t table_size;
>>>  			    const int32_t *symb_table;
>>> -# if WIDE_CHAR_VERSION
>>> -			    char str[c1];
>>> -			    unsigned int strcnt;
>>> -# else
>>> -#  define str (startp + 1)
>>> -# endif
>>> +			    struct char_array str;
>>> +			    char_array_init_empty (&str);
>>>  			    const unsigned char *extra;
>>>  			    int32_t idx;
>>>  			    int32_t elem;
>>>  			    int32_t second;
>>>  			    int32_t hash;
>>>  
>>> -# if WIDE_CHAR_VERSION
>>>  			    /* We have to convert the name to a single-byte
>>>  			       string.  This is possible since the names
>>>  			       consist of ASCII characters and the internal
>>>  			       representation is UCS4.  */
>>> -			    for (strcnt = 0; strcnt < c1; ++strcnt)
>>> -			      str[strcnt] = startp[1 + strcnt];
>>> -#endif
>>> +			    for (size_t strcnt = 0; strcnt < c1; ++strcnt)
>>> +			      char_array_append_char (&str, startp[1 + strcnt]);
>>>  
>>>  			    table_size =
>>>  			      _NL_CURRENT_WORD (LC_COLLATE,
>>
>> That needs to be removed altogether, see
>> <http://sourceware.org/ml/libc-alpha/2017-04/msg00068.html>.
> 
> Nice, I missed your patch.  It seems to apply clean, I will review on the original
> thread.

Re-checking the bug reports related to the issues your patch aims to fix, I
noted on BZ#14185 comment #5 Jeff Law stated SuSE used to pack an out of tree
fix but non-longer do it. Do you know why exactly?
Andreas Schwab Feb. 6, 2018, 8:57 a.m. UTC | #4 
On Feb 05 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> Re-checking the bug reports related to the issues your patch aims to fix, I
> noted on BZ#14185 comment #5 Jeff Law stated SuSE used to pack an out of tree
> fix but non-longer do it. Do you know why exactly?

I can't answer that question, I wasn't around at that time (the
changelog talks about "obsolete patch", which I don't quite follow).
But this bug is about matching the string, not processing the pattern,
so it is an unrelated issue.

Andreas.
diff mbox

Patch

diff --git a/posix/fnmatch.c b/posix/fnmatch.c
index 9c2cff0..b94c965 100644
--- a/posix/fnmatch.c
+++ b/posix/fnmatch.c
@@ -35,6 +35,7 @@ 
 #endif
 
 #include <scratch_buffer.h>
+#include <malloc/char_array-skeleton.c>
 
 /* For platform which support the ISO C amendement 1 functionality we
    support user defined character classes.  */
diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
index eadb343..831e5ba 100644
--- a/posix/fnmatch_loop.c
+++ b/posix/fnmatch_loop.c
@@ -493,26 +493,20 @@  FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 			  {
 			    int32_t table_size;
 			    const int32_t *symb_table;
-# if WIDE_CHAR_VERSION
-			    char str[c1];
-			    unsigned int strcnt;
-# else
-#  define str (startp + 1)
-# endif
+			    struct char_array str;
+			    char_array_init_empty (&str);
 			    const unsigned char *extra;
 			    int32_t idx;
 			    int32_t elem;
 			    int32_t second;
 			    int32_t hash;
 
-# if WIDE_CHAR_VERSION
 			    /* We have to convert the name to a single-byte
 			       string.  This is possible since the names
 			       consist of ASCII characters and the internal
 			       representation is UCS4.  */
-			    for (strcnt = 0; strcnt < c1; ++strcnt)
-			      str[strcnt] = startp[1 + strcnt];
-#endif
+			    for (size_t strcnt = 0; strcnt < c1; ++strcnt)
+			      char_array_append_char (&str, startp[1 + strcnt]);
 
 			    table_size =
 			      _NL_CURRENT_WORD (LC_COLLATE,
@@ -525,7 +519,7 @@  FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 					   _NL_COLLATE_SYMB_EXTRAMB);
 
 			    /* Locate the character in the hashing table.  */
-			    hash = elem_hash (str, c1);
+			    hash = elem_hash (char_array_str (&str), c1);
 
 			    idx = 0;
 			    elem = hash % table_size;
@@ -539,7 +533,7 @@  FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 				    if (symb_table[2 * elem] == hash
 					&& (c1
 					    == extra[symb_table[2 * elem + 1]])
-					&& memcmp (str,
+					&& memcmp (char_array_str (&str),
 						   &extra[symb_table[2 * elem
 								     + 1]
 							  + 1], c1) == 0)
@@ -580,14 +574,20 @@  FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 					break;
 
 				    if ((int32_t) c1 == wextra[idx])
-				      goto matched;
+				      {
+					char_array_free (&str);
+				        goto matched;
+				      }
 # else
 				    for (c1 = 0; c1 < extra[idx]; ++c1)
 				      if (n[c1] != extra[1 + c1])
 					break;
 
 				    if (c1 == extra[idx])
-				      goto matched;
+				      {
+					char_array_free (&str);
+				        goto matched;
+				      }
 # endif
 				  }
 
@@ -608,18 +608,21 @@  FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 			      {
 				/* No valid character.  Match it as a
 				   single byte.  */
-				if (!is_range && *n == str[0])
-				  goto matched;
+				char str0 = *char_array_at (&str, 0);
+				if (!is_range && *n == str0)
+				  {
+				    char_array_free (&str);
+				    goto matched;
+				  }
 
-				cold = str[0];
+				cold = str0;
 				c = *p++;
 			      }
-			    else
-			      return FNM_NOMATCH;
+			    char_array_free (&str);
+			    return FNM_NOMATCH;
 			  }
 		      }
 		    else
-# undef str
 #endif
 		      {
 			c = FOLD (c);
@@ -711,26 +714,20 @@  FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 			      {
 				int32_t table_size;
 				const int32_t *symb_table;
-# if WIDE_CHAR_VERSION
-				char str[c1];
-				unsigned int strcnt;
-# else
-#  define str (startp + 1)
-# endif
+				struct char_array str;
+				char_array_init_empty (&str);
 				const unsigned char *extra;
 				int32_t idx;
 				int32_t elem;
 				int32_t second;
 				int32_t hash;
 
-# if WIDE_CHAR_VERSION
 				/* We have to convert the name to a single-byte
 				   string.  This is possible since the names
 				   consist of ASCII characters and the internal
 				   representation is UCS4.  */
-				for (strcnt = 0; strcnt < c1; ++strcnt)
-				  str[strcnt] = startp[1 + strcnt];
-# endif
+				for (size_t strcnt = 0; strcnt < c1; ++strcnt)
+				  char_array_append_char (&str, startp[1 + strcnt]);
 
 				table_size =
 				  _NL_CURRENT_WORD (LC_COLLATE,
@@ -744,7 +741,7 @@  FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 
 				/* Locate the character in the hashing
 				   table.  */
-				hash = elem_hash (str, c1);
+				hash = elem_hash (char_array_str (&str), c1);
 
 				idx = 0;
 				elem = hash % table_size;
@@ -758,7 +755,7 @@  FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 					if (symb_table[2 * elem] == hash
 					    && (c1
 						== extra[symb_table[2 * elem + 1]])
-					    && memcmp (str,
+					    && memcmp (char_array_str (&str),
 						       &extra[symb_table[2 * elem + 1]
 							      + 1], c1) == 0)
 					  {
@@ -800,13 +797,12 @@  FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 				  }
 				else if (symb_table[2 * elem] != 0 && c1 == 1)
 				  {
-				    cend = str[0];
+				    cend = *char_array_at (&str, 0);
 				    c = *p++;
 				  }
-				else
-				  return FNM_NOMATCH;
+				char_array_free (&str);
+				return FNM_NOMATCH;
 			      }
-# undef str
 			  }
 			else
 			  {