Re: [PATCH] un-nest findidx()

[Konstantin Serebryany <konstantin dot s dot serebryany at gmail dot com>]

Here it is, verified against fresh trunk.

2014-09-09  Kostya Serebryany  <konstantin.s.serebryany@gmail.com>

        * locale/weight.h: add include guard.
        (findidx): un-nest, make it static inline, add parameters.
        * locale/weightwc.h: add include guard, rename findidx to findidxwc.
        (findidxwc): un-nest, make it static inline, add parameters.
        * posix/fnmatch_loop.c: include weightwc.h or weight.h depending on
        WIDE_CHAR_VERSION. Define FINDIDX as findidxwc or findidx.
        (FCT): change type of 'extra' to wint_t; do not include weight.h,
        un-nest calls to findidx.
        * posix/regcomp.c: include weight.h.
        (build_equiv_class): don't include weight.h, un-nest findidx.
        * posix/regex_internal.h: include weight.h
        (re_string_elem_size_at): don't include weight.h, un-nest findidx.
        * posix/regexec.c: include weight.h.
        (check_node_accept_bytes): don't include weight.h, un-nest findidx.
        * string/strcoll_l.c: define FINDIDX, include WEIGHT_H.
        (get_next_seq): don't include WEIGHT_H, un-nest findidx.
        (get_next_seq_nocache): don't include WEIGHT_H, un-nest findidx.
        * string/strxfrm_l.c: define FINDIDX, include WEIGHT_H.
        (STRXFRM): don't include WEIGHT_H, un-nest findidx.
        * wcsmbs/wcscoll_l.c: define FINDIDX.
        * wcsmbs/wcsxfrm_l.c: define FINDIDX.




On Tue, Sep 9, 2014 at 2:57 PM, Roland McGrath <roland@hack.frob.com> wrote:
> When it's been very long at all, you should post a fresh patch that's
> verified against today's trunk, and not include any quoted nonsense in the
> message.  Just post in the same thread and that should be enough context
> for everybody.

diff --git a/locale/weight.h b/locale/weight.h
index 9eb8ac6..485526c 100644
--- a/locale/weight.h
+++ b/locale/weight.h
@@ -16,10 +16,16 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#ifndef _WEIGHT_H_
+#define _WEIGHT_H_
+
 /* Find index of weight.  */
-auto inline int32_t
+static inline int32_t
 __attribute ((always_inline))
-findidx (const unsigned char **cpp, size_t len)
+findidx (const int32_t *table,
+	 const int32_t *indirect,
+	 const unsigned char *extra,
+	 const unsigned char **cpp, size_t len)
 {
   int_fast32_t i = table[*(*cpp)++];
   const unsigned char *cp;
@@ -130,3 +136,5 @@ findidx (const unsigned char **cpp, size_t len)
   /* NOTREACHED */
   return 0x43219876;
 }
+
+#endif  /* weight.h */
diff --git a/locale/weightwc.h b/locale/weightwc.h
index 8f047e3..3348544 100644
--- a/locale/weightwc.h
+++ b/locale/weightwc.h
@@ -16,10 +16,16 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#ifndef _WEIGHTWC_H_
+#define _WEIGHTWC_H_
+
 /* Find index of weight.  */
-auto inline int32_t
+static inline int32_t
 __attribute ((always_inline))
-findidx (const wint_t **cpp, size_t len)
+findidxwc (const int32_t *table,
+	   const int32_t *indirect,
+	   const wint_t *extra,
+	   const wint_t **cpp, size_t len)
 {
   wint_t ch = *(*cpp)++;
   int32_t i = __collidx_table_lookup ((const char *) table, ch);
@@ -109,3 +115,5 @@ findidx (const wint_t **cpp, size_t len)
   /* NOTREACHED */
   return 0x43219876;
 }
+
+#endif  /* weightwc.h */
diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
index f79d051..8cd2f31 100644
--- a/posix/fnmatch_loop.c
+++ b/posix/fnmatch_loop.c
@@ -17,6 +17,17 @@
 
 #include <stdint.h>
 
+# if WIDE_CHAR_VERSION
+#  include <locale/weightwc.h>
+#  undef FINDIDX
+#  define FINDIDX findidxwc
+# else
+#  include <locale/weight.h>
+#  undef FINDIDX
+#  define FINDIDX findidx
+# endif
+
+
 struct STRUCT
 {
   const CHAR *pattern;
@@ -376,7 +387,7 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 			const int32_t *table;
 # if WIDE_CHAR_VERSION
 			const int32_t *weights;
-			const int32_t *extra;
+			const wint_t *extra;
 # else
 			const unsigned char *weights;
 			const unsigned char *extra;
@@ -385,19 +396,12 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 			int32_t idx;
 			const UCHAR *cp = (const UCHAR *) str;
 
-			/* This #include defines a local function!  */
-# if WIDE_CHAR_VERSION
-#  include <locale/weightwc.h>
-# else
-#  include <locale/weight.h>
-# endif
-
 # if WIDE_CHAR_VERSION
 			table = (const int32_t *)
 			  _NL_CURRENT (LC_COLLATE, _NL_COLLATE_TABLEWC);
 			weights = (const int32_t *)
 			  _NL_CURRENT (LC_COLLATE, _NL_COLLATE_WEIGHTWC);
-			extra = (const int32_t *)
+			extra = (const wint_t *)
 			  _NL_CURRENT (LC_COLLATE, _NL_COLLATE_EXTRAWC);
 			indirect = (const int32_t *)
 			  _NL_CURRENT (LC_COLLATE, _NL_COLLATE_INDIRECTWC);
@@ -412,7 +416,7 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 			  _NL_CURRENT (LC_COLLATE, _NL_COLLATE_INDIRECTMB);
 # endif
 
-			idx = findidx (&cp, 1);
+			idx = FINDIDX (table, indirect, extra, &cp, 1);
 			if (idx != 0)
 			  {
 			    /* We found a table entry.  Now see whether the
@@ -422,7 +426,8 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used)
 			    int32_t idx2;
 			    const UCHAR *np = (const UCHAR *) n;
 
-			    idx2 = findidx (&np, string_end - n);
+			    idx2 = FINDIDX (table, indirect, extra,
+					    &np, string_end - n);
 			    if (idx2 != 0
 				&& (idx >> 24) == (idx2 >> 24)
 				&& len == weights[idx2 & 0xffffff])
diff --git a/posix/regcomp.c b/posix/regcomp.c
index 921d0f4..b1cc8bc 100644
--- a/posix/regcomp.c
+++ b/posix/regcomp.c
@@ -3389,6 +3389,8 @@ parse_bracket_symbol (bracket_elem_t *elem, re_string_t *regexp,
   return REG_NOERROR;
 }
 
+#include <locale/weight.h>
+
   /* Helper function for parse_bracket_exp.
      Build the equivalence class which is represented by NAME.
      The result are written to MBCSET and SBCSET.
@@ -3413,8 +3415,6 @@ build_equiv_class (bitset_t sbcset, const unsigned char *name)
       int32_t idx1, idx2;
       unsigned int ch;
       size_t len;
-      /* This #include defines a local function!  */
-# include <locale/weight.h>
       /* Calculate the index for equivalence class.  */
       cp = name;
       table = (const int32_t *) _NL_CURRENT (LC_COLLATE, _NL_COLLATE_TABLEMB);
@@ -3424,7 +3424,7 @@ build_equiv_class (bitset_t sbcset, const unsigned char *name)
 						   _NL_COLLATE_EXTRAMB);
       indirect = (const int32_t *) _NL_CURRENT (LC_COLLATE,
 						_NL_COLLATE_INDIRECTMB);
-      idx1 = findidx (&cp, -1);
+      idx1 = findidx (table, indirect, extra, &cp, -1);
       if (BE (idx1 == 0 || *cp != '\0', 0))
 	/* This isn't a valid character.  */
 	return REG_ECOLLATE;
@@ -3435,7 +3435,7 @@ build_equiv_class (bitset_t sbcset, const unsigned char *name)
 	{
 	  char_buf[0] = ch;
 	  cp = char_buf;
-	  idx2 = findidx (&cp, 1);
+	  idx2 = findidx (table, indirect, extra, &cp, 1);
 /*
 	  idx2 = table[ch];
 */
diff --git a/posix/regex_internal.h b/posix/regex_internal.h
index 75c390f..c38980b 100644
--- a/posix/regex_internal.h
+++ b/posix/regex_internal.h
@@ -733,6 +733,8 @@ re_string_wchar_at (const re_string_t *pstr, int idx)
 }
 
 # ifndef NOT_IN_libc
+# include <locale/weight.h>
+
 static int
 internal_function __attribute__ ((pure, unused))
 re_string_elem_size_at (const re_string_t *pstr, int idx)
@@ -740,7 +742,6 @@ re_string_elem_size_at (const re_string_t *pstr, int idx)
 #  ifdef _LIBC
   const unsigned char *p, *extra;
   const int32_t *table, *indirect;
-#   include <locale/weight.h>
   uint_fast32_t nrules = _NL_CURRENT_WORD (LC_COLLATE, _NL_COLLATE_NRULES);
 
   if (nrules != 0)
@@ -751,7 +752,7 @@ re_string_elem_size_at (const re_string_t *pstr, int idx)
       indirect = (const int32_t *) _NL_CURRENT (LC_COLLATE,
 						_NL_COLLATE_INDIRECTMB);
       p = pstr->mbs + idx;
-      findidx (&p, pstr->len - idx);
+      findidx (table, indirect, extra, &p, pstr->len - idx);
       return p - pstr->mbs - idx;
     }
   else
diff --git a/posix/regexec.c b/posix/regexec.c
index 7032da7..3e52682 100644
--- a/posix/regexec.c
+++ b/posix/regexec.c
@@ -3749,6 +3749,8 @@ group_nodes_into_DFAstates (const re_dfa_t *dfa, const re_dfastate_t *state,
    one collating element like '.', '[a-z]', opposite to the other nodes
    can only accept one byte.  */
 
+#include <locale/weight.h>
+
 static int
 internal_function
 check_node_accept_bytes (const re_dfa_t *dfa, int node_idx,
@@ -3868,8 +3870,6 @@ check_node_accept_bytes (const re_dfa_t *dfa, int node_idx,
 	  const int32_t *table, *indirect;
 	  const unsigned char *weights, *extra;
 	  const char *collseqwc;
-	  /* This #include defines a local function!  */
-#  include <locale/weight.h>
 
 	  /* match with collating_symbol?  */
 	  if (cset->ncoll_syms)
@@ -3925,7 +3925,7 @@ check_node_accept_bytes (const re_dfa_t *dfa, int node_idx,
 		_NL_CURRENT (LC_COLLATE, _NL_COLLATE_EXTRAMB);
 	      indirect = (const int32_t *)
 		_NL_CURRENT (LC_COLLATE, _NL_COLLATE_INDIRECTMB);
-	      int32_t idx = findidx (&cp, elem_len);
+	      int32_t idx = findidx (table, indirect, extra, &cp, elem_len);
 	      if (idx > 0)
 		for (i = 0; i < cset->nequiv_classes; ++i)
 		  {
diff --git a/string/strcoll_l.c b/string/strcoll_l.c
index 10ce4a6..8764e67 100644
--- a/string/strcoll_l.c
+++ b/string/strcoll_l.c
@@ -33,6 +33,7 @@
 # define STRCMP strcmp
 # define STRLEN strlen
 # define WEIGHT_H "../locale/weight.h"
+# define FINDIDX findidx
 # define SUFFIX	MB
 # define L(arg) arg
 #endif
@@ -146,13 +147,14 @@ get_next_seq_cached (coll_seq *seq, int nrules, int pass,
   seq->idxnow = idxnow;
 }
 
+#include WEIGHT_H
+
 /* Get next sequence.  Traverse the string as required.  */
 static void
 get_next_seq (coll_seq *seq, int nrules, const unsigned char *rulesets,
 	      const USTRING_TYPE *weights, const int32_t *table,
 	      const USTRING_TYPE *extra, const int32_t *indirect)
 {
-#include WEIGHT_H
   size_t val = seq->val = 0;
   int len = seq->len;
   size_t backw_stop = seq->backw_stop;
@@ -194,7 +196,7 @@ get_next_seq (coll_seq *seq, int nrules, const unsigned char *rulesets,
 
 	  while (*us != L('\0'))
 	    {
-	      int32_t tmp = findidx (&us, -1);
+	      int32_t tmp = FINDIDX (table, indirect, extra, &us, -1);
 	      rulearr[idxmax] = tmp >> 24;
 	      idxarr[idxmax] = tmp & 0xffffff;
 	      idxcnt = idxmax++;
@@ -242,7 +244,6 @@ get_next_seq_nocache (coll_seq *seq, int nrules, const unsigned char *rulesets,
 		      const USTRING_TYPE *extra, const int32_t *indirect,
 		      int pass)
 {
-#include WEIGHT_H
   size_t val = seq->val = 0;
   int len = seq->len;
   size_t backw_stop = seq->backw_stop;
@@ -285,7 +286,7 @@ get_next_seq_nocache (coll_seq *seq, int nrules, const unsigned char *rulesets,
 	      us = seq->back_us;
 	      while (i < backw)
 		{
-		  int32_t tmp = findidx (&us, -1);
+		  int32_t tmp = FINDIDX (table, indirect, extra, &us, -1);
 		  idx = tmp & 0xffffff;
 		  i++;
 		}
@@ -300,7 +301,7 @@ get_next_seq_nocache (coll_seq *seq, int nrules, const unsigned char *rulesets,
 
 	  while (*us != L('\0'))
 	    {
-	      int32_t tmp = findidx (&us, -1);
+	      int32_t tmp = FINDIDX (table, indirect, extra, &us, -1);
 	      unsigned char rule = tmp >> 24;
 	      prev_idx = idx;
 	      idx = tmp & 0xffffff;
diff --git a/string/strxfrm_l.c b/string/strxfrm_l.c
index 04b9338..7d41a28 100644
--- a/string/strxfrm_l.c
+++ b/string/strxfrm_l.c
@@ -33,6 +33,7 @@
 # define STRLEN strlen
 # define STPNCPY __stpncpy
 # define WEIGHT_H "../locale/weight.h"
+# define FINDIDX findidx
 # define SUFFIX	MB
 # define L(arg) arg
 #endif
@@ -80,6 +81,7 @@ utf8_encode (char *buf, int val)
 }
 #endif
 
+#include WEIGHT_H
 
 size_t
 STRXFRM (STRING_TYPE *dest, const STRING_TYPE *src, size_t n, __locale_t l)
@@ -104,8 +106,6 @@ STRXFRM (STRING_TYPE *dest, const STRING_TYPE *src, size_t n, __locale_t l)
   size_t idxcnt;
   int use_malloc;
 
-#include WEIGHT_H
-
   if (nrules == 0)
     {
       if (n != 0)
@@ -174,7 +174,7 @@ STRXFRM (STRING_TYPE *dest, const STRING_TYPE *src, size_t n, __locale_t l)
   idxmax = 0;
   do
     {
-      int32_t tmp = findidx (&usrc, -1);
+      int32_t tmp = FINDIDX (table, indirect, extra, &usrc, -1);
       rulearr[idxmax] = tmp >> 24;
       idxarr[idxmax] = tmp & 0xffffff;
 
diff --git a/wcsmbs/wcscoll_l.c b/wcsmbs/wcscoll_l.c
index 74e2e39..a402806 100644
--- a/wcsmbs/wcscoll_l.c
+++ b/wcsmbs/wcscoll_l.c
@@ -26,6 +26,7 @@
 #define STRCMP wcscmp
 #define STRLEN __wcslen
 #define WEIGHT_H "../locale/weightwc.h"
+#define FINDIDX findidxwc
 #define SUFFIX	WC
 #define L(arg) L##arg
 #define WIDE_CHAR_VERSION 1
diff --git a/wcsmbs/wcsxfrm_l.c b/wcsmbs/wcsxfrm_l.c
index f3f3f50..23e1703 100644
--- a/wcsmbs/wcsxfrm_l.c
+++ b/wcsmbs/wcsxfrm_l.c
@@ -26,6 +26,7 @@
 #define STRLEN __wcslen
 #define STPNCPY __wcpncpy
 #define WEIGHT_H "../locale/weightwc.h"
+#define FINDIDX findidxwc
 #define SUFFIX  WC
 #define L(arg) L##arg
 #define WIDE_CHAR_VERSION 1