Re: regex & g++

[Paul Eggert]

Sam Steingold <address@hidden> writes:

> a superficial code examination shows that these are indeed bugs in
> regex: build_wcs_upper_buffer should be declared to return reg_errcode_t
> and not int.

Thanks for reporting this.  I installed the following patch, which I
hope covers all the C++ problems in the regex module.  I also filed
glibc bug 1241.

2005-08-25  Paul Eggert  <address@hidden>

        Make regex safe for g++.  This fixes one real bug (an "err"
        that should have been "*err").  g++ problem reported by
        Sam Steingold.
        * config/srclist.txt: Add glibc bug 1241.
        * lib/regex_internal.h (re_calloc): New macro, consistent with
        re_malloc etc.  All callers of calloc changed to use re_calloc.
        * lib/regex_internal.c (build_wcs_upper_buffer): Return reg_errcode_t,
        not int.  All callers changed.
        * lib/regcomp.c (re_compile_fastmap_iter): Don't use alloca
        (mb_cur_max); just use an array of size MB_LEN_MAX.
        * lib/regexec.c (push_fail_stack): Use re_realloc, not realloc.
        (find_recover_state): Change "err" to "*err"; this fixes what
        appears to be a real bug.
        (check_arrival_expand_ecl_sub): Be consistent about reg_errcode_t
        versus int.

--- config/srclist.txt  25 Aug 2005 05:09:01 -0000      1.85
+++ config/srclist.txt  25 Aug 2005 20:37:51 -0000
@@ -101,6 +101,7 @@ $LIBCSRC/stdlib/getsubopt.c         lib gpl
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1224
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1237
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1240
+# http://sources.redhat.com/bugzilla/show_bug.cgi?id=1241
 #$LIBCSRC/posix/regcomp.c              lib gpl
 #
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1238
@@ -120,11 +121,13 @@ $LIBCSRC/stdlib/getsubopt.c               lib gpl
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1226
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1231
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1237
+# http://sources.redhat.com/bugzilla/show_bug.cgi?id=1241
 #$LIBCSRC/posix/regex_internal.c               lib gpl
 #
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1054
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1221
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1237
+# http://sources.redhat.com/bugzilla/show_bug.cgi?id=1241
 #$LIBCSRC/posix/regex_internal.h               lib gpl
 #
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1216
@@ -133,6 +136,7 @@ $LIBCSRC/stdlib/getsubopt.c         lib gpl
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1227
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1231
 # http://sources.redhat.com/bugzilla/show_bug.cgi?id=1237
+# http://sources.redhat.com/bugzilla/show_bug.cgi?id=1241
 #$LIBCSRC/posix/regexec.c              lib gpl
 #
 # c89 changes $LIBCSRC/string/strdup.c         lib gpl
--- lib/regex_internal.h        24 Aug 2005 23:29:39 -0000      1.3
+++ lib/regex_internal.h        25 Aug 2005 20:21:02 -0000
@@ -391,7 +391,8 @@ static reg_errcode_t re_string_realloc_b
      internal_function;
 #ifdef RE_ENABLE_I18N
 static void build_wcs_buffer (re_string_t *pstr) internal_function;
-static int build_wcs_upper_buffer (re_string_t *pstr) internal_function;
+static reg_errcode_t build_wcs_upper_buffer (re_string_t *pstr)
+     internal_function;
 #endif /* RE_ENABLE_I18N */
 static void build_upper_buffer (re_string_t *pstr) internal_function;
 static void re_string_translate_buffer (re_string_t *pstr) internal_function;
@@ -431,6 +432,7 @@ static unsigned char re_string_fetch_byt
 #define re_string_set_index(pstr,idx) ((pstr)->cur_idx = (idx))
 
 #define re_malloc(t,n) ((t *) malloc ((n) * sizeof (t)))
+#define re_calloc(t,n) ((t *) calloc (n, sizeof (t)))
 #define re_realloc(p,t,n) ((t *) realloc (p, (n) * sizeof (t)))
 #define re_free(p) free (p)
 
--- lib/regex_internal.c        24 Aug 2005 23:29:39 -0000      1.7
+++ lib/regex_internal.c        25 Aug 2005 20:21:02 -0000
@@ -258,7 +258,7 @@ build_wcs_buffer (re_string_t *pstr)
 /* Build wide character buffer PSTR->WCS like build_wcs_buffer,
    but for REG_ICASE.  */
 
-static int
+static reg_errcode_t
 internal_function
 build_wcs_upper_buffer (re_string_t *pstr)
 {
@@ -707,7 +707,7 @@ re_string_reconstruct (re_string_t *pstr
     {
       if (pstr->icase)
        {
-         int ret = build_wcs_upper_buffer (pstr);
+         reg_errcode_t ret = build_wcs_upper_buffer (pstr);
          if (BE (ret != REG_NOERROR, 0))
            return ret;
        }
@@ -1504,7 +1504,7 @@ create_ci_newstate (re_dfa_t *dfa, const
   reg_errcode_t err;
   re_dfastate_t *newstate;
 
-  newstate = (re_dfastate_t *) calloc (sizeof (re_dfastate_t), 1);
+  newstate = re_calloc (re_dfastate_t, 1);
   if (BE (newstate == NULL, 0))
     return NULL;
   err = re_node_set_init_copy (&newstate->nodes, nodes);
@@ -1554,7 +1554,7 @@ create_cd_newstate (re_dfa_t *dfa, const
   reg_errcode_t err;
   re_dfastate_t *newstate;
 
-  newstate = (re_dfastate_t *) calloc (sizeof (re_dfastate_t), 1);
+  newstate = re_calloc (re_dfastate_t, 1);
   if (BE (newstate == NULL, 0))
     return NULL;
   err = re_node_set_init_copy (&newstate->nodes, nodes);
--- lib/regcomp.c       25 Aug 2005 05:09:02 -0000      1.8
+++ lib/regcomp.c       25 Aug 2005 20:21:02 -0000
@@ -311,7 +311,8 @@ re_compile_fastmap_iter (regex_t *bufp, 
 #ifdef RE_ENABLE_I18N
          if ((bufp->re_syntax & REG_IGNORE_CASE) && dfa->mb_cur_max > 1)
            {
-             unsigned char *buf = alloca (dfa->mb_cur_max), *p;
+             unsigned char buf[MB_LEN_MAX];
+             unsigned char *p;
              wchar_t wc;
              mbstate_t state;
 
@@ -817,7 +818,7 @@ init_dfa (re_dfa_t *dfa, int pat_len)
     if (table_size > pat_len)
       break;
 
-  dfa->state_table = calloc (sizeof (struct re_state_table_entry), table_size);
+  dfa->state_table = re_calloc (struct re_state_table_entry, table_size);
   dfa->state_hash_mask = table_size - 1;
 
   dfa->mb_cur_max = MB_CUR_MAX;
@@ -860,7 +861,7 @@ init_dfa (re_dfa_t *dfa, int pat_len)
        {
          int i, j, ch;
 
-         dfa->sb_char = (re_bitset_ptr_t) calloc (sizeof (bitset), 1);
+         dfa->sb_char = re_calloc (unsigned int, BITSET_UINTS);
          if (BE (dfa->sb_char == NULL, 0))
            return REG_ESPACE;
 
@@ -2963,9 +2964,9 @@ parse_bracket_exp (re_string_t *regexp, 
                                                   _NL_COLLATE_SYMB_EXTRAMB);
     }
 #endif
-  sbcset = (re_bitset_ptr_t) calloc (sizeof (unsigned int), BITSET_UINTS);
+  sbcset = re_calloc (unsigned int, BITSET_UINTS);
 #ifdef RE_ENABLE_I18N
-  mbcset = (re_charset_t *) calloc (sizeof (re_charset_t), 1);
+  mbcset = re_calloc (re_charset_t, 1);
 #endif /* RE_ENABLE_I18N */
 #ifdef RE_ENABLE_I18N
   if (BE (sbcset == NULL || mbcset == NULL, 0))
@@ -3492,9 +3493,9 @@ build_charclass_op (re_dfa_t *dfa, unsig
   re_token_t br_token;
   bin_tree_t *tree;
 
-  sbcset = (re_bitset_ptr_t) calloc (sizeof (unsigned int), BITSET_UINTS);
+  sbcset = re_calloc (unsigned int, BITSET_UINTS);
 #ifdef RE_ENABLE_I18N
-  mbcset = (re_charset_t *) calloc (sizeof (re_charset_t), 1);
+  mbcset = re_calloc (re_charset_t, 1);
 #endif /* RE_ENABLE_I18N */
 
 #ifdef RE_ENABLE_I18N
--- lib/regexec.c       24 Aug 2005 23:29:39 -0000      1.7
+++ lib/regexec.c       25 Aug 2005 20:21:03 -0000
@@ -1306,9 +1306,8 @@ push_fail_stack (struct re_fail_stack_t 
   int num = fs->num++;
   if (fs->num == fs->alloc)
     {
-      struct re_fail_stack_ent_t *new_array;
-      new_array = realloc (fs->stack, (sizeof (struct re_fail_stack_ent_t)
-                                      * fs->alloc * 2));
+      struct re_fail_stack_ent_t *new_array =
+       re_realloc (fs->stack, struct re_fail_stack_ent_t, fs->alloc * 2);
       if (new_array == NULL)
        return REG_ESPACE;
       fs->alloc *= 2;
@@ -2326,7 +2325,7 @@ find_recover_state (reg_errcode_t *err, 
 
       cur_state = merge_state_with_log (err, mctx, NULL);
     }
-  while (err == REG_NOERROR && cur_state == NULL);
+  while (*err == REG_NOERROR && cur_state == NULL);
   return cur_state;
 }
 
@@ -2708,8 +2707,8 @@ get_subexp (re_match_context_t *mctx, in
            continue; /* No.  */
          if (sub_top->path == NULL)
            {
-             sub_top->path = calloc (sizeof (state_array_t),
-                                     sl_str - sub_top->str_idx + 1);
+             sub_top->path = re_calloc (state_array_t,
+                                        sl_str - sub_top->str_idx + 1);
              if (sub_top->path == NULL)
                return REG_ESPACE;
            }
@@ -3111,11 +3110,12 @@ check_arrival_expand_ecl_sub (re_dfa_t *
        break;
       if (dfa->edests[cur_node].nelem == 2)
        {
-         err = check_arrival_expand_ecl_sub (dfa, dst_nodes,
-                                             dfa->edests[cur_node].elems[1],
-                                             ex_subexp, type);
-         if (BE (err != REG_NOERROR, 0))
-           return err;
+         reg_errcode_t ret =
+           check_arrival_expand_ecl_sub (dfa, dst_nodes,
+                                         dfa->edests[cur_node].elems[1],
+                                         ex_subexp, type);
+         if (BE (ret != REG_NOERROR, 0))
+           return ret;
        }
       cur_node = dfa->edests[cur_node].elems[0];
     }
@@ -3263,8 +3263,7 @@ build_trtable (re_dfa_t *dfa, re_dfastat
       /* Return 0 in case of an error, 1 otherwise.  */
       if (ndests == 0)
        {
-         state->trtable = (re_dfastate_t **)
-           calloc (sizeof (re_dfastate_t *), SBC_MAX);
+         state->trtable = re_calloc (re_dfastate_t *, SBC_MAX);
          return 1;
        }
       return 0;
@@ -3352,8 +3351,7 @@ out_free:
         character, or we are in a single-byte character set so we can
         discern by looking at the character code: allocate a
         256-entry transition table.  */
-      trtable = state->trtable =
-       (re_dfastate_t **) calloc (sizeof (re_dfastate_t *), SBC_MAX);
+      trtable = state->trtable = re_calloc (re_dfastate_t *, SBC_MAX);
       if (BE (trtable == NULL, 0))
        goto out_free;
 
@@ -3383,8 +3381,7 @@ out_free:
         by looking at the character code: build two 256-entry
         transition tables, one starting at trtable[0] and one
         starting at trtable[SBC_MAX].  */
-      trtable = state->word_trtable =
-       (re_dfastate_t **) calloc (sizeof (re_dfastate_t *), 2 * SBC_MAX);
+      trtable = state->word_trtable = re_calloc (re_dfastate_t *, 2 * SBC_MAX);
       if (BE (trtable == NULL, 0))
        goto out_free;
 
@@ -4204,7 +4201,7 @@ match_ctx_add_subtop (re_match_context_t
       mctx->sub_tops = new_array;
       mctx->asub_tops = new_asub_tops;
     }
-  mctx->sub_tops[mctx->nsub_tops] = calloc (1, sizeof (re_sub_match_top_t));
+  mctx->sub_tops[mctx->nsub_tops] = re_calloc (re_sub_match_top_t, 1);
   if (BE (mctx->sub_tops[mctx->nsub_tops] == NULL, 0))
     return REG_ESPACE;
   mctx->sub_tops[mctx->nsub_tops]->node = node;
@@ -4231,7 +4228,7 @@ match_ctx_add_sublast (re_sub_match_top_
       subtop->lasts = new_array;
       subtop->alasts = new_alasts;
     }
-  new_entry = calloc (1, sizeof (re_sub_match_last_t));
+  new_entry = re_calloc (re_sub_match_last_t, 1);
   if (BE (new_entry != NULL, 1))
     {
       subtop->lasts[subtop->nlasts] = new_entry;