| Message ID | 290ba923b5ee5bcaa4801454b6692deb532bd681.1587740959.git.congdanhqx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix Sparse Warning | expand |
On 24/04/2020 16:12, Đoàn Trần Công Danh wrote: > Somewhere later in the code, we indirectly include alloca.h > which will define alloca again, thus create a warning about > redefinition of a preprocessor. > > Include it prior to define alloca in order to not define it again. So, on cygwin, this patch is not required. ie. I don't see any sparse errors/warnings for compat/regex/regex.c. Since cygwin uses a different c library (new-lib rather than glibc), I did a quick test on Linux, thus: $ sparse --version v0.6.1-191-gc51a0382 $ $ git checkout master Switched to branch 'master' Your branch is up-to-date with 'origin/master'. $ $ make clean GIT_VERSION = 2.26.2.266.ge870325ee8 ... $ $ make NO_REGEX=1 sparse >sp-out1 2>&1 $ $ diff sp-out sp-out1 0a1,2 > GIT_VERSION = 2.26.2.266.ge870325ee8 > * new build flags 12a15 > * new prefix flags 72a76 > GEN command-list.h 226a231 > SP compat/regex/regex.c $ $ make V=1 NO_REGEX=1 compat/regex/regex.sp cgcc -no-compile -Werror -Wall -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -DENABLE_SHA256 -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU="\"x86_64\"" -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH="/proc/self/exe"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"' -DGAWK -DNO_MBSUPPORT \ compat/regex/regex.c $ So, again I don't see a problem. I guess it is possible that the version of sparse I am using (see above) has _also_ fixed this problem, in addition to the prototype attribute placement fix. Another option is that the version of glibc also matters. (I am on Linux Mint, which is based on Ubuntu 18.04 LTS) It would not be the first time that I have seen errors in system header files change from one release to the next ... [Hmm, I have a fedora 31 system I could try - much more up-to-date! :D ] ATB, Ramsay Jones > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > --- > compat/regex/regex.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/compat/regex/regex.c b/compat/regex/regex.c > index f3e03a9eab..4bef75a716 100644 > --- a/compat/regex/regex.c > +++ b/compat/regex/regex.c > @@ -62,6 +62,7 @@ > #include <stdint.h> > > #ifdef GAWK > +#include <alloca.h> > #undef alloca > #define alloca alloca_is_bad_you_should_never_use_it > #endif >
On 2020-04-24 17:56:46+0100, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > > > On 24/04/2020 16:12, Đoàn Trần Công Danh wrote: > > Somewhere later in the code, we indirectly include alloca.h > > which will define alloca again, thus create a warning about > > redefinition of a preprocessor. > > > > Include it prior to define alloca in order to not define it again. > > So, on cygwin, this patch is not required. ie. I don't see any sparse > errors/warnings for compat/regex/regex.c. > > Since cygwin uses a different c library (new-lib rather than glibc), > I did a quick test on Linux, thus: > > $ sparse --version > v0.6.1-191-gc51a0382 > $ > $ git checkout master > Switched to branch 'master' > Your branch is up-to-date with 'origin/master'. > $ > $ make clean > GIT_VERSION = 2.26.2.266.ge870325ee8 > ... > $ > $ make NO_REGEX=1 sparse >sp-out1 2>&1 > $ > $ diff sp-out sp-out1 > 0a1,2 > > GIT_VERSION = 2.26.2.266.ge870325ee8 > > * new build flags > 12a15 > > * new prefix flags > 72a76 > > GEN command-list.h > 226a231 > > SP compat/regex/regex.c > $ > $ make V=1 NO_REGEX=1 compat/regex/regex.sp > cgcc -no-compile -Werror -Wall -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -DENABLE_SHA256 -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU="\"x86_64\"" -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH="/proc/self/exe"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"' -DGAWK -DNO_MBSUPPORT \ > compat/regex/regex.c > $ > > So, again I don't see a problem. I guess it is possible that the > version of sparse I am using (see above) has _also_ fixed this > problem, in addition to the prototype attribute placement fix. > > Another option is that the version of glibc also matters. (I am > on Linux Mint, which is based on Ubuntu 18.04 LTS) It would not > be the first time that I have seen errors in system header files > change from one release to the next ... I'm using a Linux distro with musl libc. I guess it's the main culprit? I have another box with glibc, but it's mostly in Windows 10, because my sister is its main user. I'll take a look if it make that warning when my sister agree to leave that box to me.
On 24/04/2020 18:09, Danh Doan wrote: > On 2020-04-24 17:56:46+0100, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: [snip] >> So, again I don't see a problem. I guess it is possible that the >> version of sparse I am using (see above) has _also_ fixed this >> problem, in addition to the prototype attribute placement fix. >> >> Another option is that the version of glibc also matters. (I am >> on Linux Mint, which is based on Ubuntu 18.04 LTS) It would not >> be the first time that I have seen errors in system header files >> change from one release to the next ... > > I'm using a Linux distro with musl libc. Ah, OK. I just tried re-building v0.6.1 to see if any '<alloca.h>' related errors/warnings show up for me, but they don't: $ sparse --version v0.6.1 $ $ git checkout master Switched to branch 'master' Your branch is up-to-date with 'origin/master'. $ $ make clean GIT_VERSION = 2.26.2.266.ge870325ee8 ... $ $ make NO_REGEX=1 compat/regex/regex.sp SP compat/regex/regex.c compat/regex/regex_internal.c:925:1: error: symbol 're_string_context_at' redeclared with different type (originally declared at compat/regex/regex_internal.h:433) - different modifiers $ > I guess it's the main culprit? Quite possible, I guess. What do the errors/warnings look like? > I have another box with glibc, but it's mostly in Windows 10, > because my sister is its main user. > > I'll take a look if it make that warning when my sister agree to leave > that box to me. Sounds good. ATB, Ramsay Jones
On 2020-04-24 19:29:31+0100, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > > > On 24/04/2020 18:09, Danh Doan wrote: > > On 2020-04-24 17:56:46+0100, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > [snip] > > >> So, again I don't see a problem. I guess it is possible that the > >> version of sparse I am using (see above) has _also_ fixed this > >> problem, in addition to the prototype attribute placement fix. > >> > >> Another option is that the version of glibc also matters. (I am > >> on Linux Mint, which is based on Ubuntu 18.04 LTS) It would not > >> be the first time that I have seen errors in system header files > >> change from one release to the next ... > > > > I'm using a Linux distro with musl libc. > > Ah, OK. > > I just tried re-building v0.6.1 to see if any '<alloca.h>' related > errors/warnings show up for me, but they don't: > > $ sparse --version > v0.6.1 > $ > $ git checkout master > Switched to branch 'master' > Your branch is up-to-date with 'origin/master'. > $ > $ make clean > GIT_VERSION = 2.26.2.266.ge870325ee8 > ... > $ > $ make NO_REGEX=1 compat/regex/regex.sp > SP compat/regex/regex.c > compat/regex/regex_internal.c:925:1: error: symbol 're_string_context_at' redeclared with different type (originally declared at compat/regex/regex_internal.h:433) - different modifiers > $ > > > I guess it's the main culprit? > > Quite possible, I guess. What do the errors/warnings look like? OK, I've tried with my glibc box, it doesn't have that warning. On musl, it warns: $ make compat/regex/regex.sp GIT_VERSION = 2.26.2 * new build flags SP compat/regex/regex.c /usr/include/alloca.h:14:9: warning: preprocessor token alloca redefined compat/regex/regex.c:66:9: this was the original definition compat/regex/regex_internal.c:925:1: error: symbol 're_string_context_at' redeclared with different type (originally declared at compat/regex/regex_internal.h:433) - different modifiers
On 24/04/2020 23:34, Danh Doan wrote: [snip] > OK, I've tried with my glibc box, it doesn't have that warning. > On musl, it warns: > > $ make compat/regex/regex.sp > GIT_VERSION = 2.26.2 > * new build flags > SP compat/regex/regex.c > /usr/include/alloca.h:14:9: warning: preprocessor token alloca redefined > compat/regex/regex.c:66:9: this was the original definition > compat/regex/regex_internal.c:925:1: error: symbol 're_string_context_at' redeclared with different type (originally declared at compat/regex/regex_internal.h:433) - different modifiers > > OK, I had a quick look at the <alloca.h> header file on a glibc system (linux) and new-lib system (cygwin) and they both do (more or less) the same thing: first #undef alloca, and then if being compiled by gcc, define alloca(size) to be __builtin_alloca(size). So, even if <alloca.h> is #included after regex.c:66, it wouldn't be a problem. Since I don't have access to a musl based system, I don't know what that system header is doing. However, I said *even if* above, because I don't see why it is trying to #include <alloca.h> in the first place! ;-) Note that the three calls to alloca in compat/regex: $ git grep -n '\<alloca\>' -- compat/regex compat/regex/regex.c:65:#undef alloca compat/regex/regex.c:66:#define alloca alloca_is_bad_you_should_never_use_it compat/regex/regex_internal.h:460:# include <alloca.h> compat/regex/regex_internal.h:468:/* alloca is implemented with malloc, so just use malloc. */ compat/regex/regexec.c:1428: prev_idx_match = (regmatch_t *) alloca (nmatch * sizeof (regmatch_t)); compat/regex/regexec.c:3338: dests_alloc = (struct dests_alloc *) alloca (sizeof (struct dests_alloc)); compat/regex/regexec.c:3385: alloca (ndests * 3 * sizeof (re_dfastate_t *)); $ ... in compat/regex/regexec.c are all protected by '#ifdef HAVE_ALLOCA', as indeed is the #include of the header file in compat/regex/regex_internal.h at line 460. Since HAVE_ALLOCA should not be defined, where is that file being included? Note that HAVE_ALLOCA $ git grep -n HAVE_ALLOCA -- compat compat/regex/regex_internal.h:455:# if HAVE_ALLOCA compat/regex/regexec.c:1426:#ifdef HAVE_ALLOCA compat/regex/regexec.c:3336:#ifdef HAVE_ALLOCA compat/regex/regexec.c:3381:#ifdef HAVE_ALLOCA $ ... is not the same as HAVE_ALLOCA_H $ git grep -n HAVE_ALLOCA Makefile:45:# Define HAVE_ALLOCA_H if you have working alloca(3) defined in that header. Makefile:1349:ifdef HAVE_ALLOCA_H Makefile:1350: BASIC_CFLAGS += -DHAVE_ALLOCA_H compat/regex/regex_internal.h:455:# if HAVE_ALLOCA compat/regex/regexec.c:1426:#ifdef HAVE_ALLOCA compat/regex/regexec.c:3336:#ifdef HAVE_ALLOCA compat/regex/regexec.c:3381:#ifdef HAVE_ALLOCA config.mak.uname:47: HAVE_ALLOCA_H = YesPlease config.mak.uname:63: HAVE_ALLOCA_H = YesPlease config.mak.uname:147: HAVE_ALLOCA_H = YesPlease config.mak.uname:206: HAVE_ALLOCA_H = YesPlease config.mak.uname:310: HAVE_ALLOCA_H = YesPlease config.mak.uname:395: HAVE_ALLOCA_H = YesPlease config.mak.uname:586: HAVE_ALLOCA_H = YesPlease configure.ac:320:# Define HAVE_ALLOCA_H if you have working alloca(3) defined in that header. configure.ac:323: yes) HAVE_ALLOCA_H=YesPlease;; configure.ac:324: *) HAVE_ALLOCA_H='';; configure.ac:326:GIT_CONF_SUBST([HAVE_ALLOCA_H]) git-compat-util.h:840:#ifdef HAVE_ALLOCA_H $ ... used by the rest of git, outside of the compat/regex directory. Once again, you can see that HAVE_ALLOCA $ make V=1 NO_REGEX=1 compat/regex/regex.sp cgcc -no-compile -Werror -Wall -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -DENABLE_SHA256 -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU="\"x86_64\"" -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH="/proc/self/exe"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"' -DGAWK -DNO_MBSUPPORT \ compat/regex/regex.c $ ... is not being set on the command-line. Hmm, do you have this set in config.mak, config.mak.autogen, or some other source? puzzled! ;-) BTW, why are you compiling with NO_REGEX set anyway? ATB, Ramsay Jones
On 2020-04-25 21:28:05+0100, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > > > On 24/04/2020 23:34, Danh Doan wrote: > [snip] > > > OK, I've tried with my glibc box, it doesn't have that warning. > > On musl, it warns: > > > > $ make compat/regex/regex.sp > > GIT_VERSION = 2.26.2 > > * new build flags > > SP compat/regex/regex.c > > /usr/include/alloca.h:14:9: warning: preprocessor token alloca redefined > > compat/regex/regex.c:66:9: this was the original definition > > compat/regex/regex_internal.c:925:1: error: symbol 're_string_context_at' redeclared with different type (originally declared at compat/regex/regex_internal.h:433) - different modifiers > > > > > > OK, I had a quick look at the <alloca.h> header file on a glibc > system (linux) and new-lib system (cygwin) and they both do > (more or less) the same thing: first #undef alloca, and then > if being compiled by gcc, define alloca(size) to be __builtin_alloca(size). musl people don't do that. They just go ahead define it, if any other header file requires alloca, they will include alloca.h > So, even if <alloca.h> is #included after regex.c:66, it wouldn't > be a problem. Since I don't have access to a musl based system, > I don't know what that system header is doing. musl's alloca.h is available here: https://git.musl-libc.org/cgit/musl/tree/include/alloca.h > However, I said *even if* above, because I don't see why it is trying > to #include <alloca.h> in the first place! ;-) I looked into my system again. The inclusion chain is: compat/regex/regex.c:77 `-> compat/regex/regex_internal.h:26 `-> /usr/include/stdlib.h:138 [*1*] [*1*]: https://git.musl-libc.org/cgit/musl/tree/include/stdlib.h#n137 I don't know why _GNU_SOURCE and/or _BSD_SOURCE is defined. > $ make V=1 NO_REGEX=1 compat/regex/regex.sp > cgcc -no-compile -Werror -Wall -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -DENABLE_SHA256 -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU="\"x86_64\"" -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH="/proc/self/exe"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"' -DGAWK -DNO_MBSUPPORT \ > compat/regex/regex.c > $ > > ... is not being set on the command-line. Here's the invocation of cc and cgcc: $ make V=1 compat/regex/regex.o cc -o compat/regex/regex.o -c -MF compat/regex/.depend/regex.o.d -MQ compat/regex/regex.o -MMD -MP -Werror -Wall -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -DENABLE_SHA256 -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU="\"x86_64\"" -DUSE_LIBPCRE2 -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DICONV_OMITS_BOM -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH="/proc/self/exe"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"' -DGAWK -DNO_MBSUPPORT compat/regex/regex.c $ make V=1 compat/regex/regex.sp cgcc -no-compile -Werror -Wall -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -DENABLE_SHA256 -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU="\"x86_64\"" -DUSE_LIBPCRE2 -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DICONV_OMITS_BOM -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH="/proc/self/exe"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"' -DGAWK -DNO_MBSUPPORT \ compat/regex/regex.c > Hmm, do you have this set in config.mak, config.mak.autogen, or some other > source? puzzled! ;-) I don't have `config.make.autogen`, Here is config.mak $ cat config.mak USE_ASCIIDOCTOR=Yes DEVELOPER=1 DEFAULT_TEST_TARGET=prove prefix = /home/danh/.local USE_LIBPCRE2=YesPlease ICONV_OMITS_BOM=Yes NO_REGEX=YesPlease > BTW, why are you compiling with NO_REGEX set anyway? Because I use musl-libc, and musl-libc doesn't have StartEnd
On 2020-04-26 07:54:51+0700, Danh Doan <congdanhqx@gmail.com> wrote: > On 2020-04-25 21:28:05+0100, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > > > > > > On 24/04/2020 23:34, Danh Doan wrote: > > [snip] > > > > > OK, I've tried with my glibc box, it doesn't have that warning. > > > On musl, it warns: > > > > > > $ make compat/regex/regex.sp > > > GIT_VERSION = 2.26.2 > > > * new build flags > > > SP compat/regex/regex.c > > > /usr/include/alloca.h:14:9: warning: preprocessor token alloca redefined > > > compat/regex/regex.c:66:9: this was the original definition > > > compat/regex/regex_internal.c:925:1: error: symbol 're_string_context_at' redeclared with different type (originally declared at compat/regex/regex_internal.h:433) - different modifiers > > > > > > > > > > OK, I had a quick look at the <alloca.h> header file on a glibc > > system (linux) and new-lib system (cygwin) and they both do > > (more or less) the same thing: first #undef alloca, and then > > if being compiled by gcc, define alloca(size) to be __builtin_alloca(size). > > musl people don't do that. > They just go ahead define it, if any other header file requires > alloca, they will include alloca.h > > > So, even if <alloca.h> is #included after regex.c:66, it wouldn't > > be a problem. Since I don't have access to a musl based system, > > I don't know what that system header is doing. > > musl's alloca.h is available here: > > https://git.musl-libc.org/cgit/musl/tree/include/alloca.h > > > However, I said *even if* above, because I don't see why it is trying > > to #include <alloca.h> in the first place! ;-) > > I looked into my system again. The inclusion chain is: > > compat/regex/regex.c:77 > `-> compat/regex/regex_internal.h:26 > `-> /usr/include/stdlib.h:138 [*1*] > `-> /usr/include/alloca.h > > [*1*]: https://git.musl-libc.org/cgit/musl/tree/include/stdlib.h#n137 Sorry for the noise, but I should link to a specific tree instead of their master for future refererence. https://git.musl-libc.org/cgit/musl/tree/include/stdlib.h?id=8e452abae67db445fb6c3e37cd566c4788c2e8f3#n137 > > I don't know why _GNU_SOURCE and/or _BSD_SOURCE is defined. > > > $ make V=1 NO_REGEX=1 compat/regex/regex.sp > > cgcc -no-compile -Werror -Wall -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -DENABLE_SHA256 -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU="\"x86_64\"" -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH="/proc/self/exe"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"' -DGAWK -DNO_MBSUPPORT \ > > compat/regex/regex.c > > $ > > > > ... is not being set on the command-line. > > Here's the invocation of cc and cgcc: > > $ make V=1 compat/regex/regex.o > cc -o compat/regex/regex.o -c -MF compat/regex/.depend/regex.o.d -MQ compat/regex/regex.o -MMD -MP -Werror -Wall -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -DENABLE_SHA256 -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU="\"x86_64\"" -DUSE_LIBPCRE2 -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DICONV_OMITS_BOM -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH="/proc/self/exe"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"' -DGAWK -DNO_MBSUPPORT compat/regex/regex.c > $ make V=1 compat/regex/regex.sp > cgcc -no-compile -Werror -Wall -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -DENABLE_SHA256 -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU="\"x86_64\"" -DUSE_LIBPCRE2 -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DICONV_OMITS_BOM -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH="/proc/self/exe"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"' -DGAWK -DNO_MBSUPPORT \ > compat/regex/regex.c > > > Hmm, do you have this set in config.mak, config.mak.autogen, or some other > > source? puzzled! ;-) > > I don't have `config.make.autogen`, > Here is config.mak > > $ cat config.mak > USE_ASCIIDOCTOR=Yes > DEVELOPER=1 > DEFAULT_TEST_TARGET=prove > prefix = /home/danh/.local > USE_LIBPCRE2=YesPlease > ICONV_OMITS_BOM=Yes > NO_REGEX=YesPlease > > > BTW, why are you compiling with NO_REGEX set anyway? > > Because I use musl-libc, and musl-libc doesn't have StartEnd > > -- > Danh
On 26/04/2020 01:54, Danh Doan wrote: > On 2020-04-25 21:28:05+0100, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: [snip] >> OK, I had a quick look at the <alloca.h> header file on a glibc >> system (linux) and new-lib system (cygwin) and they both do >> (more or less) the same thing: first #undef alloca, and then >> if being compiled by gcc, define alloca(size) to be __builtin_alloca(size). > > musl people don't do that. > They just go ahead define it, if any other header file requires > alloca, they will include alloca.h > >> So, even if <alloca.h> is #included after regex.c:66, it wouldn't >> be a problem. Since I don't have access to a musl based system, >> I don't know what that system header is doing. > > musl's alloca.h is available here: > > https://git.musl-libc.org/cgit/musl/tree/include/alloca.h Hmm, OK, so that partly explains the problem. I wonder if the musl guys would accept a bug report? >> However, I said *even if* above, because I don't see why it is trying >> to #include <alloca.h> in the first place! ;-) > > I looked into my system again. The inclusion chain is: > > compat/regex/regex.c:77 > `-> compat/regex/regex_internal.h:26 > `-> /usr/include/stdlib.h:138 [*1*] > > [*1*]: https://git.musl-libc.org/cgit/musl/tree/include/stdlib.h#n137 > > I don't know why _GNU_SOURCE and/or _BSD_SOURCE is defined. ... and this explains the main cause. Hmm, as you say, why are one (or both) of those pp variables set. :( [snip] >> BTW, why are you compiling with NO_REGEX set anyway? > > Because I use musl-libc, and musl-libc doesn't have StartEnd Ah, OK. ;-) Well, even if the musl project accepted a PR and provided a fix, that will not help you in the short term! :D 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', ... ;-) ATB, Ramsay Jones
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
Ramsay Jones <ramsay@ramsayjones.plus.com> writes: > 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 This does smell like a right approach to me. If we can get it tested widely, that would be good. Thanks. > -- >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
On 2020-04-26 17:17:56+0100, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > > > On 26/04/2020 01:54, Danh Doan wrote: > > On 2020-04-25 21:28:05+0100, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > [snip] > >> OK, I had a quick look at the <alloca.h> header file on a glibc > >> system (linux) and new-lib system (cygwin) and they both do > >> (more or less) the same thing: first #undef alloca, and then > >> if being compiled by gcc, define alloca(size) to be __builtin_alloca(size). > > > > musl people don't do that. > > They just go ahead define it, if any other header file requires > > alloca, they will include alloca.h > > > >> So, even if <alloca.h> is #included after regex.c:66, it wouldn't > >> be a problem. Since I don't have access to a musl based system, > >> I don't know what that system header is doing. > > > > musl's alloca.h is available here: > > > > https://git.musl-libc.org/cgit/musl/tree/include/alloca.h > > Hmm, OK, so that partly explains the problem. I wonder if the > musl guys would accept a bug report? I don't think they have a policy of no `#undef` whatsoever. But, I think they're picky when come to C-correctly and POSIX-correctly. Does C or POSIX define alloca(3) at all? And, I /think/ they'll likely ignore this one, [musl-faq] says: Assuming that including one header will cause symbols from another unrelated header to be exposed. This is an application bug, and fixing it is as simple as adding the missing #include directives. I guess they meant, if we have `alloca` defined, we must have included `alloca.h` somewhere. > >> However, I said *even if* above, because I don't see why it is trying > >> to #include <alloca.h> in the first place! ;-) > > > > I looked into my system again. The inclusion chain is: > > > > compat/regex/regex.c:77 > > `-> compat/regex/regex_internal.h:26 > > `-> /usr/include/stdlib.h:138 [*1*] > > > > [*1*]: https://git.musl-libc.org/cgit/musl/tree/include/stdlib.h#n137 > > > > I don't know why _GNU_SOURCE and/or _BSD_SOURCE is defined. > > ... and this explains the main cause. Hmm, as you say, why are > one (or both) of those pp variables set. :( Okay, I tracked it down. compat/regex/regex.c:63 `-> /usr/include/limits.h:4 `-> /usr/include/features.h:15 https://git.musl-libc.org/cgit/musl/tree/include/features.h?id=8e452abae67db445fb6c3e37cd566c4788c2e8f3#n14 musl defined `_BSD_SOURCE` if none of those below macro was defined: - _BSD_SOURCE - _POSIX_SOURCE - _XOPEN_SOURCE - _GNU_SOURCE - __STRICT_ANSI__ I don't think we have any business to define which one of those macros should be defined in the compat code. > > [snip] > >> BTW, why are you compiling with NO_REGEX set anyway? > > > > Because I use musl-libc, and musl-libc doesn't have StartEnd > > Ah, OK. ;-) > > Well, even if the musl project accepted a PR and provided a fix, that > will not help you in the short term! :D > > 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. musl's wiki's faq [musl-wiki-faq]: Q: Why is there no __MUSL__ macro? > - 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 ... I'll go with your patch in the next email. <1a0c2b25-e283-9936-1fa2-ce51df1404dc@ramsayjones.plus.com> [musl-faq]: https://www.musl-libc.org/faq.html [musl-wiki-faq]: https://wiki.musl-libc.org/faq.html
On 27/04/2020 02:08, Danh Doan wrote: [snip] >>> musl's alloca.h is available here: >>> >>> https://git.musl-libc.org/cgit/musl/tree/include/alloca.h >> >> Hmm, OK, so that partly explains the problem. I wonder if the >> musl guys would accept a bug report? > > I don't think they have a policy of no `#undef` whatsoever. That's fair enough. > But, I think they're picky when come to C-correctly and > POSIX-correctly. > Does C or POSIX define alloca(3) at all? No alloca() is not in either the POSIX or C standard(s). This was an extension from the early days of BSD Unix. For some reason, I thought you had to explicitly '#include <alloca.h>' to use it, but it appears that (by default) you get a bonus include from the <stdlib.h> header, unless you restrict the headers using the various macros and/or compiler command-line options. As it happens, even on glibc systems, the <alloca.h> header is included by the <stdlib.h> header, unless you take steps to suppress it. So, we would have had the same issue, if it wasn't for the aforementioned '#undef alloca' the the glibc header file. When I need to look at pp output, while debugging things like this, I cherry-pick a patch to the Makefile: $ git diff diff --git a/Makefile b/Makefile index 6d5cee270c..cd8753bf54 100644 --- a/Makefile +++ b/Makefile @@ -2423,6 +2423,9 @@ $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) %.s: %.c GIT-CFLAGS FORCE $(QUIET_CC)$(CC) -o $@ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $< +%.i: %.c GIT-CFLAGS FORCE + $(QUIET_CC)$(CC) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) -E $< >$*.i + ifdef USE_COMPUTED_HEADER_DEPENDENCIES # Take advantage of gcc's on-the-fly dependency generation # See <http://gcc.gnu.org/gcc-3.0/features.html>. @@ -2474,7 +2477,7 @@ http-walker.sp http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT endif ifdef NO_REGEX -compat/regex/regex.sp compat/regex/regex.o: EXTRA_CPPFLAGS = \ +compat/regex/regex.i compat/regex/regex.sp compat/regex/regex.o: EXTRA_CPPFLAGS = \ -DGAWK -DNO_MBSUPPORT endif $ [The second hunk above is not actually part of the cherry-picked patch, but I needed it in this instance to get GAWK and NO_MBSUPPORT passed to the compiler!] $ make NO_REGEX=1 compat/regex/regex.i CC compat/regex/regex.i $ vim compat/regex/regex.i $ ... which shows <alloca.h> is indeed being #included from <stdlib.h>. [it is protected by a __USE_MISC pp variable, but I didn't bother to track it down! ;-)] ATB, Ramsay Jones
On 2020-04-27 17:28:03+0100, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > No alloca() is not in either the POSIX or C standard(s). > This was an extension from the early days of BSD Unix. Okay, I saw it was mentioned in GNU manpage for alloca(3) > For some reason, I thought you had to explicitly '#include <alloca.h>' > to use it, but it appears that (by default) you get a bonus include > from the <stdlib.h> header, unless you restrict the headers using the > various macros and/or compiler command-line options. It looks like <alloca.h> is GNU's invention. *BSD defined it in <stdlib.h>: https://www.freebsd.org/cgi/man.cgi?alloca https://man.openbsd.org/alloca https://netbsd.gw.com/cgi-bin/man-cgi?alloca+3+NetBSD-current Even GNU's alloca(3) says: Normally, gcc(1) translates calls to alloca() with inlined code. This is not done when either the -ansi, -std=c89, -std=c99, or the -std=c11 option is given and the header <alloca.h> is not included. Otherwise, (without an -ansi or -std=c* option) the glibc version of <stdlib.h> includes <alloca.h> > As it happens, even on glibc systems, the <alloca.h> header is included > by the <stdlib.h> header, unless you take steps to suppress it. So, we > would have had the same issue, if it wasn't for the aforementioned > '#undef alloca' the the glibc header file. From above information, I think it's fine to include <stdlib.h> first. It's AT&T Unix's invention and everyone seems to follow it (except Windows, but the lack of complains from our Windows friends may signify that their alloca is fine already). I've sent it already for v3. > When I need to look at pp output, while debugging things like this, > I cherry-pick a patch to the Makefile: > > $ git diff > diff --git a/Makefile b/Makefile > index 6d5cee270c..cd8753bf54 100644 > --- a/Makefile > +++ b/Makefile > @@ -2423,6 +2423,9 @@ $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) > %.s: %.c GIT-CFLAGS FORCE > $(QUIET_CC)$(CC) -o $@ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $< > > +%.i: %.c GIT-CFLAGS FORCE > + $(QUIET_CC)$(CC) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) -E $< >$*.i > + > ifdef USE_COMPUTED_HEADER_DEPENDENCIES > # Take advantage of gcc's on-the-fly dependency generation > # See <http://gcc.gnu.org/gcc-3.0/features.html>. > @@ -2474,7 +2477,7 @@ http-walker.sp http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT > endif > > ifdef NO_REGEX > -compat/regex/regex.sp compat/regex/regex.o: EXTRA_CPPFLAGS = \ > +compat/regex/regex.i compat/regex/regex.sp compat/regex/regex.o: EXTRA_CPPFLAGS = \ > -DGAWK -DNO_MBSUPPORT > endif > > $ I think it's worth to have this included. `.s` rules is there, anyway. Personally, I've run this to trace the inclusion: make V=1 CFLAGS=-E compat/regex/regex.o compat/regex/regex.sp
On 27/04/2020 17:46, Danh Doan wrote: [snip] > It looks like <alloca.h> is GNU's invention. > > *BSD defined it in <stdlib.h>: > https://www.freebsd.org/cgi/man.cgi?alloca > https://man.openbsd.org/alloca > https://netbsd.gw.com/cgi-bin/man-cgi?alloca+3+NetBSD-current Yeah, that would make sense. It's been about 25 years since I used a BSD based system (Hmm, Irix changed its base from AT&T to BSD at one point; or was it the other way round - I forget!) >> As it happens, even on glibc systems, the <alloca.h> header is included >> by the <stdlib.h> header, unless you take steps to suppress it. So, we >> would have had the same issue, if it wasn't for the aforementioned >> '#undef alloca' the the glibc header file. > >>From above information, I think it's fine to include <stdlib.h> first. > It's AT&T Unix's invention and everyone seems to follow it (except Windows, > but the lack of complains from our Windows friends may signify that > their alloca is fine already). > > I've sent it already for v3. Yep, looks good. >> When I need to look at pp output, while debugging things like this, >> I cherry-pick a patch to the Makefile: >> >> $ git diff >> diff --git a/Makefile b/Makefile >> index 6d5cee270c..cd8753bf54 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -2423,6 +2423,9 @@ $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) >> %.s: %.c GIT-CFLAGS FORCE >> $(QUIET_CC)$(CC) -o $@ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $< >> >> +%.i: %.c GIT-CFLAGS FORCE >> + $(QUIET_CC)$(CC) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) -E $< >$*.i >> + >> ifdef USE_COMPUTED_HEADER_DEPENDENCIES >> # Take advantage of gcc's on-the-fly dependency generation >> # See <http://gcc.gnu.org/gcc-3.0/features.html>. >> @@ -2474,7 +2477,7 @@ http-walker.sp http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT >> endif >> >> ifdef NO_REGEX >> -compat/regex/regex.sp compat/regex/regex.o: EXTRA_CPPFLAGS = \ >> +compat/regex/regex.i compat/regex/regex.sp compat/regex/regex.o: EXTRA_CPPFLAGS = \ >> -DGAWK -DNO_MBSUPPORT >> endif >> >> $ > > I think it's worth to have this included. `.s` rules is there, anyway. Hmm, I can't remember if I ever actually submitted a patch; I've had this patch floating in my git repo for about 10 years or so! ;-) I don't use it very often, but it's very useful when needed. ATB, Ramsay Jones
diff --git a/compat/regex/regex.c b/compat/regex/regex.c index f3e03a9eab..4bef75a716 100644 --- a/compat/regex/regex.c +++ b/compat/regex/regex.c @@ -62,6 +62,7 @@ #include <stdint.h> #ifdef GAWK +#include <alloca.h> #undef alloca #define alloca alloca_is_bad_you_should_never_use_it #endif
Somewhere later in the code, we indirectly include alloca.h which will define alloca again, thus create a warning about redefinition of a preprocessor. Include it prior to define alloca in order to not define it again. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- compat/regex/regex.c | 1 + 1 file changed, 1 insertion(+)