Re: [PATCH v2 2/4] compat/regex: include alloca.h before undef it

[Ramsay Jones <ramsay@ramsayjones.plus.com>]

On 26/04/2020 17:17, Ramsay Jones wrote:
[snip]

> Hmm, whatever patch you decide to send (even the original one) I think
> you need to add an explanation of the problem to the commit message,
> including why the patch provides a solution. (You don't have to type
> a novel - see commit bd8f005583 :-P ).
> 
> I haven't thought about this too much, but some options:
> 
>   - iff the musl library sets some kind of identifying pp variable
>     (_MUSL_LIBC_ or somesuch - I haven't looked), then you could
>     make the '#include <alloca.h>' conditional on that variable.
>     This has the benefit of making it obvious to people reading the
>     code that this is specific to musl-libc.
> 
>   - you could simply remove that '#ifdef GAWK' block completely (Lines
>     64->67). We set GAWK and NO_MBSUPPORT  unconditionally in the Makefile
>     so that it compiles (see commit a997bf423d), but these particular
>     lines simply reflect the gawk projects dislike of alloca (along with
>     the desire to catch any attempts to add new calls which are not protected
>     by HAVE_ALLOCA). Given that we are very unlikely to add new calls ...
> 
>   - change the conditional on this block to (totally untested, just typing
>     into my email client) '#if defined(GAWK) && defined(HAVE_ALLOCA)'.
>     This should work, but it does disable the 'catch any attempts to add
>     new _unintentional_ calls' aspect of that block; so you may as well
>     remove it ...
> 
> Just some 'off the top of my head ideas', ... ;-)

Another option I thought about, but didn't mention above, is given by
the patch below. I didn't mention it because it has the potential to
cause problems on non musl-libc systems (and I was feeling too lazy at
the time to go and test ...). Again, see commit bd8f005583.

So, I have now tested this patch on (glibc) linux and it seems to work
just fine; compile, sparse, test-suite passed. It also compiles clean
(and sparse clean) on cygwin (new-lib) - I didn't run the test-suite
on cygwin, since it takes about 3.5 to 4 hours to run.

I don't have any other systems to test this on, so I can't say that it
won't cause problems somewhere. In practice, I think the chances of that
are rather low, but don't quote me on that! :-P

ATB,
Ramsay Jones

-- >8 --
Subject: [PATCH] regex: fix up musl-libc builds

---
 compat/regex/regex.c          | 1 +
 compat/regex/regex_internal.h | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/compat/regex/regex.c b/compat/regex/regex.c
index f3e03a9eab..e6f4a5d177 100644
--- a/compat/regex/regex.c
+++ b/compat/regex/regex.c
@@ -60,6 +60,7 @@
    #undefs RE_DUP_MAX and sets it to the right value.  */
 #include <limits.h>
 #include <stdint.h>
+#include <stdlib.h>
 
 #ifdef GAWK
 #undef alloca
diff --git a/compat/regex/regex_internal.h b/compat/regex/regex_internal.h
index 3ee8aae59d..0bad8b841e 100644
--- a/compat/regex/regex_internal.h
+++ b/compat/regex/regex_internal.h
@@ -23,7 +23,6 @@
 #include <assert.h>
 #include <ctype.h>
 #include <stdio.h>
-#include <stdlib.h>
 #include <string.h>
 
 #if defined HAVE_LANGINFO_H || defined HAVE_LANGINFO_CODESET || defined _LIBC
-- 
2.26.2