diff mbox

[02/17] Regex: Fix alloca ifdefs and usage.

Message ID 201712080916.vB89GxVP005493@skeeve.com
State New, archived
Headers

Commit Message

Arnold Robbins Dec. 8, 2017, 9:16 a.m. UTC 
  This patch cleans up the use of alloca, making sure includes and
uses are inside the proper ifdef, and replacing use of
alloca with more standard constructs where that works.
Missing ifdefs are added.

2017-11-27         Arnold D. Robbins     <arnold@skeeve.com>

	* posix/regcomp.c (re_compile_fastmap_iter): Replace
	alloca call with allocation on the stack.
	* posix/regex_internal.h: Fix HAVE_ALLOCA and add it where needed.
	* posix/regexec.c (set_regs): Ditto.
	(build_trtable): Ditto.

Comments

Paul Eggert Dec. 19, 2017, 10:13 p.m. UTC | #1 
For this one, the patch to regcomp.c is in Gnulib and looks good. 
However, the patch to regex_internal.h has problems compared to a 
similar patch that is already in Gnullib. That is, instead of this:
> -#include <alloca.h>
> -
>   #ifndef _LIBC
>   # if HAVE_ALLOCA
> +#  include <alloca.h>

I suggest this replacing the #include line with this:

#if defined _LIBC || HAVE_ALLOCA
# include <alloca.h>
#endif

That way, we shouldn't need to have ugly changes like this one:

> +#ifdef HAVE_ALLOCA
>     if (__libc_use_alloca (nmatch * sizeof (regmatch_t)))
>       prev_idx_match = (regmatch_t *) alloca (nmatch * sizeof (regmatch_t));
>     else
> +#endif
which will keep things simpler.
diff mbox

Patch

diff --git a/posix/regcomp.c b/posix/regcomp.c
index 6bc4102..5bb1d11 100644
--- a/posix/regcomp.c
+++ b/posix/regcomp.c
@@ -317,7 +317,8 @@  re_compile_fastmap_iter (regex_t *bufp, const re_dfastate_t *init_state,
 #ifdef RE_ENABLE_I18N
 	  if ((bufp->syntax & RE_ICASE) && 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;
 
diff --git a/posix/regex_internal.h b/posix/regex_internal.h
index fef1d35..79e438a 100644
--- a/posix/regex_internal.h
+++ b/posix/regex_internal.h
@@ -402,10 +402,9 @@  static unsigned int re_string_context_at (const re_string_t *input, int idx,
 #define re_string_skip_bytes(pstr,idx) ((pstr)->cur_idx += (idx))
 #define re_string_set_index(pstr,idx) ((pstr)->cur_idx = (idx))
 
-#include <alloca.h>
-
 #ifndef _LIBC
 # if HAVE_ALLOCA
+#  include <alloca.h>
 /* The OS usually guarantees only one guard page at the bottom of the stack,
    and a page size can be as small as 4096 bytes.  So we cannot safely
    allocate anything larger than 4096 bytes.  Also care for the possibility
diff --git a/posix/regexec.c b/posix/regexec.c
index 95e31d3..1290576 100644
--- a/posix/regexec.c
+++ b/posix/regexec.c
@@ -1383,9 +1383,11 @@  set_regs (const regex_t *preg, const re_match_context_t *mctx, size_t nmatch,
   cur_node = dfa->init_node;
   re_node_set_init_empty (&eps_via_nodes);
 
+#ifdef HAVE_ALLOCA
   if (__libc_use_alloca (nmatch * sizeof (regmatch_t)))
     prev_idx_match = (regmatch_t *) alloca (nmatch * sizeof (regmatch_t));
   else
+#endif
     {
       prev_idx_match = re_malloc (regmatch_t, nmatch);
       if (prev_idx_match == NULL)
@@ -3269,9 +3271,11 @@  build_trtable (const re_dfa_t *dfa, re_dfastate_t *state)
      from `state'.  `dests_node[i]' represents the nodes which i-th
      destination state contains, and `dests_ch[i]' represents the
      characters which i-th destination state accepts.  */
+#ifdef HAVE_ALLOCA
   if (__libc_use_alloca (sizeof (struct dests_alloc)))
     dests_alloc = (struct dests_alloc *) alloca (sizeof (struct dests_alloc));
   else
+#endif
     {
       dests_alloc = re_malloc (struct dests_alloc, 1);
       if (BE (dests_alloc == NULL, 0))
@@ -3314,11 +3318,13 @@  build_trtable (const re_dfa_t *dfa, re_dfastate_t *state)
 	  0))
     goto out_free;
 
+#ifdef HAVE_ALLOCA
   if (__libc_use_alloca ((sizeof (re_node_set) + sizeof (bitset_t)) * SBC_MAX
 			 + ndests * 3 * sizeof (re_dfastate_t *)))
     dest_states = (re_dfastate_t **)
       alloca (ndests * 3 * sizeof (re_dfastate_t *));
   else
+#endif
     {
       dest_states = (re_dfastate_t **)
 	malloc (ndests * 3 * sizeof (re_dfastate_t *));