git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ramsay Jones <ramsay@ramsayjones.plus.com>
To: Danh Doan <congdanhqx@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 2/4] compat/regex: include alloca.h before undef it
Date: Sat, 25 Apr 2020 21:28:05 +0100	[thread overview]
Message-ID: <ffcaaf4f-a43f-2d20-b70d-dfb8b1d7c687@ramsayjones.plus.com> (raw)
In-Reply-To: <20200424223440.GC721@danh.dev>



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


  reply	other threads:[~2020-04-25 20:28 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-23 13:47 [PATCH 0/4] fix make sparse warning Đoàn Trần Công Danh
2020-04-23 13:47 ` [PATCH 1/4] C: s/0/NULL/ for pointer type Đoàn Trần Công Danh
2020-04-24  0:39   ` Ramsay Jones
2020-04-24  0:54     ` Junio C Hamano
2020-04-24  1:09       ` Danh Doan
2020-04-24  1:54         ` Junio C Hamano
2020-05-14 21:37       ` Luc Van Oostenryck
2020-04-23 13:47 ` [PATCH 2/4] compat/regex: silence `make sparse` warning Đoàn Trần Công Danh
2020-04-24  0:51   ` Ramsay Jones
2020-04-24  1:04     ` Danh Doan
2020-04-23 13:47 ` [PATCH 3/4] graph.c: limit linkage of internal variable Đoàn Trần Công Danh
2020-04-24  0:52   ` Ramsay Jones
2020-04-23 13:47 ` [PATCH 4/4] progress.c: silence cgcc suggestion about internal linkage Đoàn Trần Công Danh
2020-04-24  0:58   ` Ramsay Jones
2020-04-24  5:54     ` Jeff King
2020-04-23 13:47 ` [PATCH 5/4] fmt-merge-msg.c: fix `make sparse` on next Đoàn Trần Công Danh
2020-04-23 23:10 ` [PATCH 0/4] fix make sparse warning Ramsay Jones
2020-04-23 23:58   ` Danh Doan
2020-04-24 16:38     ` Ramsay Jones
2020-04-24 15:12 ` [PATCH v2 0/4] Fix Sparse Warning Đoàn Trần Công Danh
2020-04-24 15:12   ` [PATCH v2 1/4] test-parse-pathspec-file.c: s/0/NULL/ for pointer type Đoàn Trần Công Danh
2020-04-24 15:12   ` [PATCH v2 2/4] compat/regex: include alloca.h before undef it Đoàn Trần Công Danh
2020-04-24 16:56     ` Ramsay Jones
2020-04-24 17:09       ` Danh Doan
2020-04-24 18:29         ` Ramsay Jones
2020-04-24 22:34           ` Danh Doan
2020-04-25 20:28             ` Ramsay Jones [this message]
2020-04-26  0:54               ` Danh Doan
2020-04-26  1:10                 ` Danh Doan
2020-04-26 16:17                 ` Ramsay Jones
2020-04-26 19:38                   ` Ramsay Jones
2020-04-26 22:37                     ` Junio C Hamano
2020-04-27  1:08                   ` Danh Doan
2020-04-27 16:28                     ` Ramsay Jones
2020-04-27 16:46                       ` Danh Doan
2020-04-27 17:21                         ` Ramsay Jones
2020-04-24 15:12   ` [PATCH v2 3/4] graph.c: limit linkage of internal variable Đoàn Trần Công Danh
2020-04-24 15:12   ` [PATCH v2 4/4] progress.c: silence cgcc suggestion about internal linkage Đoàn Trần Công Danh
2020-04-24 16:40   ` [PATCH v2 0/4] Fix Sparse Warning Ramsay Jones
2020-04-25 13:13 ` [PATCH 0/4] fix make sparse warning Johannes Schindelin
2020-04-26  3:32   ` Danh Doan
2020-04-26 16:24     ` Ramsay Jones
2020-05-01 20:02       ` Johannes Schindelin
2020-04-27 14:22 ` [PATCH v3 0/4] Partial fix `make sparse` Đoàn Trần Công Danh
2020-04-27 14:22   ` [PATCH v3 1/4] test-parse-pathspec-file.c: s/0/NULL/ for pointer type Đoàn Trần Công Danh
2020-04-27 14:22   ` [PATCH v3 2/4] compat/regex: move stdlib.h up in inclusion chain Đoàn Trần Công Danh
2020-04-27 16:41     ` Ramsay Jones
2020-04-27 14:22   ` [PATCH v3 3/4] graph.c: limit linkage of internal variable Đoàn Trần Công Danh
2020-04-27 14:22   ` [PATCH v3 4/4] progress.c: silence cgcc suggestion about internal linkage Đoàn Trần Công Danh
2020-05-01 20:09     ` Johannes Schindelin
2020-04-27 16:35   ` [PATCH v3 0/4] Partial fix `make sparse` Ramsay Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ffcaaf4f-a43f-2d20-b70d-dfb8b1d7c687@ramsayjones.plus.com \
    --to=ramsay@ramsayjones.plus.com \
    --cc=congdanhqx@gmail.com \
    --cc=git@vger.kernel.org \
    --subject='Re: [PATCH v2 2/4] compat/regex: include alloca.h before undef it' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).