patch, 4 partsC: s/0/NULL/ for pointer type

> 
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>  add-interactive.c                   | 2 +-
>  builtin/fmt-merge-msg.c             | 2 +-
>  log-tree.c                          | 4 ++--
>  range-diff.c                        | 2 +-
>  t/helper/test-parse-pathspec-file.c | 6 +++---
>  5 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/add-interactive.c b/add-interactive.c
> index 29cd2fe020..b8983838b9 100644
> --- a/add-interactive.c
> +++ b/add-interactive.c
> @@ -526,7 +526,7 @@ static int get_modified_files(struct repository *r,
>  
>  	for (i = 0; i < 2; i++) {
>  		struct rev_info rev;
> -		struct setup_revision_opt opt = { 0 };
> +		struct setup_revision_opt opt = { NULL };
>  
>  		if (filter == INDEX_ONLY)
>  			s.mode = (i == 0) ? FROM_INDEX : FROM_WORKTREE;
> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
> index 172dfbd852..f4376bccef 100644
> --- a/builtin/fmt-merge-msg.c
> +++ b/builtin/fmt-merge-msg.c
> @@ -494,7 +494,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
>  		enum object_type type;
>  		unsigned long size, len;
>  		char *buf = read_object_file(oid, &type, &size);
> -		struct signature_check sigc = { 0 };
> +		struct signature_check sigc = { NULL };
>  		struct strbuf sig = STRBUF_INIT;
>  
>  		if (!buf || type != OBJ_TAG)
> diff --git a/log-tree.c b/log-tree.c
> index 0064788b25..ca721150d4 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -449,7 +449,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
>  {
>  	struct strbuf payload = STRBUF_INIT;
>  	struct strbuf signature = STRBUF_INIT;
> -	struct signature_check sigc = { 0 };
> +	struct signature_check sigc = { NULL };
>  	int status;
>  
>  	if (parse_signed_commit(commit, &payload, &signature) <= 0)
> @@ -496,7 +496,7 @@ static int show_one_mergetag(struct commit *commit,
>  	struct object_id oid;
>  	struct tag *tag;
>  	struct strbuf verify_message;
> -	struct signature_check sigc = { 0 };
> +	struct signature_check sigc = { NULL };
>  	int status, nth;
>  	size_t payload_size;
>  
> diff --git a/range-diff.c b/range-diff.c
> index f745567cf6..71dcd947c5 100644
> --- a/range-diff.c
> +++ b/range-diff.c
> @@ -107,7 +107,7 @@ static int read_patches(const char *range, struct string_list *list,
>  		}
>  
>  		if (starts_with(line, "diff --git")) {
> -			struct patch patch = { 0 };
> +			struct patch patch = { NULL };
>  			struct strbuf root = STRBUF_INIT;
>  			int linenr = 0;
>  
> diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
> index 02f4ccfd2a..b3e08cef4b 100644
> --- a/t/helper/test-parse-pathspec-file.c
> +++ b/t/helper/test-parse-pathspec-file.c
> @@ -6,7 +6,7 @@
>  int cmd__parse_pathspec_file(int argc, const char **argv)
>  {
>  	struct pathspec pathspec;
> -	const char *pathspec_from_file = 0;
> +	const char *pathspec_from_file = NULL;
>  	int pathspec_file_nul = 0, i;
>  
>  	static const char *const usage[] = {
> @@ -20,9 +20,9 @@ int cmd__parse_pathspec_file(int argc, const char **argv)
>  		OPT_END()
>  	};
>  
> -	parse_options(argc, argv, 0, options, usage, 0);
> +	parse_options(argc, argv, NULL, options, usage, 0);
>  
> -	parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
> +	parse_pathspec_file(&pathspec, 0, 0, NULL, pathspec_from_file,
>  			    pathspec_file_nul);
>  
>  	for (i = 0; i < pathspec.nr; i++)
> 

> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
> > Actually, I have a patch somewhere which suppressed the sparse warning
> > for the '= { 0 }' token sequence used in these initializations. However,
> > I don't seem to be able to find them at the moment! :(
> 
> AHHHHHhhhhhhhh.  
> 
> Thanks for reminding.  Yeah, I recall that the concensus of those
> who were vocal in that old discussion [*1*] was that "= { 0 }"
> should be taken as an idiom and should not be subject to s/0/NULL/
> conversion.

> > [Luc, this topic came up on the sparse and kernel mailing-lists at one
> > point, but I didn't get around to posting my patch to the list - something
> > came up. Hopefully, I will find some time to find it and post it soon.]
> 
> 
> [References]
> 
> *1*
> 
> https://lore.kernel.org/git/e6796c60-a870-e761-3b07-b680f934c537@ramsayjones.plus.com/

> +Cc: Dscho and Han-Wen
>
>> > [Luc, this topic came up on the sparse and kernel mailing-lists at one
>> > point, but I didn't get around to posting my patch to the list - something
>> > came up. Hopefully, I will find some time to find it and post it soon.]
>> 
>> 
>> [References]
>> 
>> *1*
>> 
>> https://lore.kernel.org/git/e6796c60-a870-e761-3b07-b680f934c537@ramsayjones.plus.com/

> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
> > Actually, I have a patch somewhere which suppressed the sparse warning
> > for the '= { 0 }' token sequence used in these initializations. However,
> > I don't seem to be able to find them at the moment! :(
> 
> AHHHHHhhhhhhhh.  
> 
> Thanks for reminding.  Yeah, I recall that the concensus of those
> who were vocal in that old discussion [*1*] was that "= { 0 }"
> should be taken as an idiom and should not be subject to s/0/NULL/
> conversion.
> 
> > [Luc, this topic came up on the sparse and kernel mailing-lists at one
> > point, but I didn't get around to posting my patch to the list - something
> > came up. Hopefully, I will find some time to find it and post it soon.]
> 
> 
> [References]
> 
> *1*
> 
> https://lore.kernel.org/git/e6796c60-a870-e761-3b07-b680f934c537@ramsayjones.plus.com/

> * alloca: somewhere later in the code, we indirectly include alloca.h
> which will define alloca again, include it prior to define alloca in
> order to not define it against.
> 
> * Copy all attributes from the header to source file, and move the
>   attributes prior to function name. cgcc is very picky on the position
>   of attribute.

> 
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>  compat/regex/regex.c          | 1 +
>  compat/regex/regex_internal.c | 2 +-
>  compat/regex/regex_internal.h | 5 ++---
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> 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
> diff --git a/compat/regex/regex_internal.c b/compat/regex/regex_internal.c
> index ec51cf3446..58504f795b 100644
> --- a/compat/regex/regex_internal.c
> +++ b/compat/regex/regex_internal.c
> @@ -921,7 +921,7 @@ re_string_destruct (re_string_t *pstr)
>  /* Return the context at IDX in INPUT.  */
>  
>  static unsigned int
> -internal_function
> +internal_function __attribute ((pure))
>  re_string_context_at (const re_string_t *input, int idx, int eflags)
>  {
>    int c;
> diff --git a/compat/regex/regex_internal.h b/compat/regex/regex_internal.h
> index 3ee8aae59d..c61a1e4971 100644
> --- a/compat/regex/regex_internal.h
> +++ b/compat/regex/regex_internal.h
> @@ -430,9 +430,8 @@ static reg_errcode_t build_wcs_upper_buffer (re_string_t *pstr)
>  # 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;
> -static unsigned int re_string_context_at (const re_string_t *input, int idx,
> -					  int eflags)
> -     internal_function __attribute ((pure));
> +static internal_function __attribute ((pure))
> +unsigned int re_string_context_at (const re_string_t *input, int idx, int eflags);
>  #endif
>  #define re_string_peek_byte(pstr, offset) \
>    ((pstr)->mbs[(pstr)->cur_idx + offset])
> 

> 
> 
> On 23/04/2020 14:47, Đoàn Trần Công Danh wrote:
> > * alloca: somewhere later in the code, we indirectly include alloca.h
> > which will define alloca again, include it prior to define alloca in
> > order to not define it against.
> > 
> > * Copy all attributes from the header to source file, and move the
> >   attributes prior to function name. cgcc is very picky on the position
> >   of attribute.
> 
> This patch is no longer needed (iff you use a bleeding edge version of
> sparse), since this was fixed in commit 172f6a98 ("let function definition inherit prototype attributes", 2019-11-19).
> 
>   $ git describe 172f6a98
>   v0.6.1-37-g172f6a98
>   $ 
> 
> Ah, so this fix is not in any released version of sparse (v0.6.1 is the
> current release - I always build/run the tip of the 'master' branch from
> the sparse git repo).

> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>  graph.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/graph.c b/graph.c
> index 4fb25ad464..4cd9915075 100644
> --- a/graph.c
> +++ b/graph.c
> @@ -1055,7 +1055,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
>  		graph_update_state(graph, GRAPH_COLLAPSING);
>  }
>  
> -const char merge_chars[] = {'/', '|', '\\'};
> +static const char merge_chars[] = {'/', '|', '\\'};
>  
>  static void graph_output_post_merge_line(struct git_graph *graph, struct graph_line *line)
>  {
> 

> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>  progress.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/progress.c b/progress.c
> index 19805ac646..fb53a2ec0c 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -50,7 +50,9 @@ static volatile sig_atomic_t progress_update;
>   * These are only intended for testing the progress output, i.e. exclusively
>   * for 'test-tool progress'.
>   */
> +extern int progress_testing; /* to silence sparse: internal linkage */
>  int progress_testing;
> +extern uint64_t progress_test_ns; /* to silence sparse: internal linkage */
>  uint64_t progress_test_ns = 0;
>  void progress_test_force_update(void); /* To silence -Wmissing-prototypes */
>  void progress_test_force_update(void)

> > +extern int progress_testing; /* to silence sparse: internal linkage */
> >  int progress_testing;
> > +extern uint64_t progress_test_ns; /* to silence sparse: internal linkage */
> >  uint64_t progress_test_ns = 0;
> >  void progress_test_force_update(void); /* To silence -Wmissing-prototypes */
> >  void progress_test_force_update(void)
> 
> My preference would be to add these extern declarations to the
> progress.h header file, with a note that they are only used by
> the 't/helper/test-progress.c' test helper. (Also, remove the
> extern declarations from test-progress.c, of course).

> It's conflicted when merge down next and pu.
> 
> For next, please ignore the conflict with fmt-merge-msg.c and
> apply [5/4] instead.
> 
> For pu, reftable.c has too much warnings, I'm too lazy to look into it.

> 
> 
> On 23/04/2020 14:47, Đoàn Trần Công Danh wrote:
> > It's happened that I tried to run make check on project that support autoconf.
> 
> I try to ignore autoconf as much as possible, so I don't know why people

> > Git's Makefile told me to run `make sparse` instead.
> 
> In which case, you must have had sparse installed, otherwise you would
> have been directed to 'make test'. [See commit 912f9980d2 ("Makefile: help
> people who run 'make check' by mistake", 2008-11-11)]
> 
> The 'check' target was introduced in commit 44c9e8594e ("Fix up header file
> dependencies and add sparse checking rules", 2005-07-03). As you can see,
> that target has nothing to do with autoconf/configure.
> 
> Exactly a year later, support for autoconf tools was added in commit
> 556677144b ("autoconf: Use autoconf to write installation
> directories to config.mak.autogen", 2006-07-03). This provides an
> 'alternative' to just
> using the Makefile (which is the _primary_ method used to build git).
> 
> I suspect that the majority of git developers don't use the autoconf
> tools (I have no numbers, just gut feeling).

> I added the 'sparse' target in commit 0bcd9ae85d ("sparse: Fix
> errors due to missing target-specific variables", 2011-04-21). After
> nine years, we could
> perhaps just drop the 'check' target altogether? dunno.
> 
> > I /think/ if we have a rule in Makefile, we should adhere to it.
> 
> Hmm, do you mean that 'make check' should be _reserved_ for autoconf use?
> [fun fact: the test target in the sparse Makefile is 'check' :-D ]

> > I also fix another change in ds/blame-on-bloom, which I think it's worth to
> > fix, see: <20200423133937.GA1984@danh.dev>
> 
> Yes, looks good.
> 
> > This series is developed from latest master.
> 
> I have some comments on the patches (I'm running out of time here,
> so I may not get to them till tomorrow).
> 
> > It's conflicted when merge down next and pu.
> > 
> > For next, please ignore the conflict with fmt-merge-msg.c and
> > apply [5/4] instead.
> > 
> > For pu, reftable.c has too much warnings, I'm too lazy to look into it.
> 
> For those interested, the 'too much warnings' on the 'pu' branch looks
> like (for pu@faf094bf11):
> 
>   $ make sparse >psp-out 2>&1
>   $ diff nsp-out psp-out
>   154a155
>   >     SP refs/reftable-backend.c
>   406a408
>   >     SP t/helper/test-proc-receive.c
>   447a450,511
>   >     SP reftable/basics.c
>   > reftable/basics.c:157:6: warning: symbol 'reftable_malloc_ptr' was not declared. Should it be static?
>   > reftable/basics.c:158:6: warning: symbol 'reftable_realloc_ptr' was not declared. Should it be static?
>   > reftable/basics.c:159:6: warning: symbol 'reftable_free_ptr' was not declared. Should it be static?
>   >     SP reftable/block.c
>   >     SP reftable/bytes.c
>   >     SP reftable/file.c
>   > reftable/file.c:57:37: warning: symbol 'file_vtable' was not declared. Should it be static?
>   >     SP reftable/iter.c
>   > reftable/iter.c:32:33: warning: symbol 'empty_vtable' was not declared. Should it be static?
>   > reftable/iter.c:61:31: warning: Using plain integer as NULL pointer
>   > reftable/iter.c:69:31: warning: Using plain integer as NULL pointer
>   > reftable/iter.c:96:57: warning: Using plain integer as NULL pointer
>   > reftable/iter.c:124:33: warning: symbol 'filtering_ref_iterator_vtable' was not declared. Should it be static?
>   > reftable/iter.c:224:33: warning: symbol 'indexed_table_ref_iter_vtable' was not declared. Should it be static?
>   >     SP reftable/merged.c
>   > reftable/merged.c:141:33: warning: symbol 'merged_iter_vtable' was not declared. Should it be static?
>   > reftable/merged.c:283:31: warning: Using plain integer as NULL pointer
>   > reftable/merged.c:296:31: warning: Using plain integer as NULL pointer
>   >     SP reftable/pq.c
>   >     SP reftable/reader.c
>   > reftable/reader.c:180:42: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:181:42: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:284:41: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:360:44: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:396:33: warning: symbol 'table_iter_vtable' was not declared. Should it be static?
>   > reftable/reader.c:450:36: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:498:42: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:500:44: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:501:42: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:502:36: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:565:34: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:610:31: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:623:31: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:669:36: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:670:42: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:671:35: warning: Using plain integer as NULL pointer
>   > reftable/reader.c:672:35: warning: Using plain integer as NULL pointer
>   >     SP reftable/record.c
>   > reftable/record.c:556:22: warning: symbol 'obj_record_vtable' was not declared. Should it be static?
>   > reftable/record.c:839:22: warning: symbol 'reftable_log_record_vtable' was not declared. Should it be static?
>   > reftable/record.c:961:22: warning: symbol 'index_record_vtable' was not declared. Should it be static?
>   >     SP reftable/slice.c
>   > reftable/slice.c:180:37: warning: symbol 'slice_vtable' was not declared. Should it be static?
>   > reftable/slice.c:200:37: warning: symbol 'malloc_vtable' was not declared. Should it be static?
>   > reftable/slice.c:204:30: warning: symbol 'malloc_block_source_instance' was not declared. Should it be static?
>   >     SP reftable/stack.c
>   > reftable/stack.c:157:62: warning: Using plain integer as NULL pointer
>   > reftable/stack.c:667:41: warning: Using plain integer as NULL pointer
>   > reftable/stack.c:668:44: warning: Using plain integer as NULL pointer
>   > reftable/stack.c:669:44: warning: Using plain integer as NULL pointer
>   > reftable/stack.c:1085:41: warning: Using plain integer as NULL pointer
>   > reftable/stack.c:1111:41: warning: Using plain integer as NULL pointer
>   >     SP reftable/tree.c
>   >     SP reftable/writer.c
>   > reftable/writer.c:231:31: warning: Using plain integer as NULL pointer
>   > reftable/writer.c:299:39: warning: Using plain integer as NULL pointer
>   > reftable/writer.c:347:47: warning: Using plain integer as NULL pointer
>   > reftable/writer.c:426:31: warning: Using plain integer as NULL pointer
>   > reftable/writer.c:468:45: warning: Using plain integer as NULL pointer
>   > reftable/writer.c:584:11: warning: symbol 'debug' was not declared. Should it be static?
>   >     SP reftable/zlib-compat.c
> 
> Also, my static-check perl script says the following symbols are not
> used outside the file which defines them (so they could be marked static):

>   $ ./static-check.pl >psc
>   $ diff nsc psc
>   71a72,136
>   > reftable/basics.o	- reftable_free_ptr
>   > reftable/basics.o	- reftable_malloc_ptr
>   > reftable/basics.o	- reftable_realloc_ptr
>   > reftable/basics.o	- reftable_set_alloc
>   > reftable/block.o	- block_reader_seek
>   > reftable/block.o	- block_writer_register_restart
>   > reftable/file.o	- file_vtable
>   > reftable/iter.o	- empty_vtable
>   > reftable/iter.o	- filtering_ref_iterator_vtable
>   > reftable/iter.o	- indexed_table_ref_iter_vtable
>   > reftable/merged.o	- merged_iter_vtable
>   > reftable/merged.o	- reftable_merged_table_max_update_index
>   > reftable/merged.o	- reftable_merged_table_min_update_index
>   > reftable/merged.o	- reftable_merged_table_seek_log_at
>   > reftable/pq.o	- merged_iter_pqueue_check
>   > reftable/pq.o	- pq_less
>   > reftable/reader.o	- block_source_close
>   > reftable/reader.o	- block_source_read_block
>   > reftable/reader.o	- block_source_size
>   > reftable/reader.o	- init_reader
>   > reftable/reader.o	- reftable_reader_hash_id
>   > reftable/reader.o	- reftable_reader_refs_for
>   > reftable/reader.o	- reftable_reader_seek_log
>   > reftable/reader.o	- reftable_reader_seek_log_at
>   > reftable/reader.o	- table_iter_vtable
>   > reftable/record.o	- get_var_int
>   > reftable/record.o	- index_record_vtable
>   > reftable/record.o	- obj_record_vtable
>   > reftable/record.o	- put_var_int
>   > reftable/record.o	- record_as_log
>   > reftable/record.o	- record_as_ref
>   > reftable/record.o	- reftable_log_record_equal
>   > reftable/record.o	- reftable_log_record_print
>   > reftable/record.o	- reftable_log_record_vtable
>   > reftable/record.o	- reftable_ref_record_equal
>   > reftable/record.o	- reftable_ref_record_print
>   > reftable/record.o	- reftable_ref_record_vtable
>   > reftable/slice.o	- block_source_from_slice
>   > reftable/slice.o	- malloc_block_source_instance
>   > reftable/slice.o	- malloc_vtable
>   > reftable/slice.o	- slice_equal
>   > reftable/slice.o	- slice_vtable
>   > reftable/slice.o	- slice_write
>   > reftable/slice.o	- slice_write_void
>   > reftable/slice.o	- slice_yield
>   > reftable/stack.o	- fastlog2
>   > reftable/stack.o	- read_lines
>   > reftable/stack.o	- reftable_addition_add
>   > reftable/stack.o	- reftable_addition_close
>   > reftable/stack.o	- reftable_addition_commit
>   > reftable/stack.o	- reftable_stack_auto_compact
>   > reftable/stack.o	- reftable_stack_compaction_stats
>   > reftable/stack.o	- reftable_stack_destroy
>   > reftable/stack.o	- reftable_stack_new_addition
>   > reftable/stack.o	- reftable_stack_read_log
>   > reftable/stack.o	- reftable_stack_reload
>   > reftable/stack.o	- sizes_to_segments
>   > reftable/stack.o	- stack_try_add
>   > reftable/stack.o	- stack_write_compact
>   > reftable/stack.o	- suggest_compaction_segment
>   > reftable/writer.o	- debug
>   > reftable/writer.o	- writer_clear_index
>   > reftable/writer.o	- writer_finish_public_section
>   > reftable/writer.o	- writer_flush_block
>   > reftable/writer.o	- writer_stats
> 
> Note: I have not looked at any of the reftable patches/files.

> On 2020-04-24 00:10:25+0100, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>> On 23/04/2020 14:47, Đoàn Trần Công Danh wrote:
>>> It's happened that I tried to run make check on project that support autoconf.
>>
>> I try to ignore autoconf as much as possible, so I don't know why people
> 
> For some reason, people refer to autotools as autohell
> 
>> who use it regularly seem to expect a 'make check' target (is that anything
>> to do with ./config.status --recheck?).
> 
> Because it's autotools's recommendation.
> https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Basic-Installation.html#Basic-Installation
> https://www.gnu.org/software/automake/manual/automake.html#Standard-Targets

>> For those interested, the 'too much warnings' on the 'pu' branch looks
>> like (for pu@faf094bf11):
>>
>>   $ make sparse >psp-out 2>&1
>>   $ diff nsp-out psp-out
>>   154a155
>>   >     SP refs/reftable-backend.c
>>   406a408
>>   >     SP t/helper/test-proc-receive.c
>>   447a450,511
>>   >     SP reftable/basics.c
>>   > reftable/basics.c:157:6: warning: symbol 'reftable_malloc_ptr' was not declared. Should it be static?
>>   > reftable/basics.c:158:6: warning: symbol 'reftable_realloc_ptr' was not declared. Should it be static?
>>   > reftable/basics.c:159:6: warning: symbol 'reftable_free_ptr' was not declared. Should it be static?

>>   >     SP reftable/writer.c
>>   > reftable/writer.c:231:31: warning: Using plain integer as NULL pointer
>>   > reftable/writer.c:299:39: warning: Using plain integer as NULL pointer
>>   > reftable/writer.c:347:47: warning: Using plain integer as NULL pointer
>>   > reftable/writer.c:426:31: warning: Using plain integer as NULL pointer
>>   > reftable/writer.c:468:45: warning: Using plain integer as NULL pointer
>>   > reftable/writer.c:584:11: warning: symbol 'debug' was not declared. Should it be static?
>>   >     SP reftable/zlib-compat.c
>>
>> Also, my static-check perl script says the following symbols are not
>> used outside the file which defines them (so they could be marked static):
> 
> All of them are in reftable, and they're catched by `make sparse`,
> too.

> 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(+)
> 
> 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 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 ...

>> 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 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?

> 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).

>   $ 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.

> 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

> 
> 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

>> 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.

> 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', ... ;-)

> -- >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 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 ...

>>> 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.

> 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
>    
>   $ 

> 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

>> 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.

> 
> Đoàn Trần Công Danh (4):
>   test-parse-pathspec-file.c: s/0/NULL/ for pointer type
>   compat/regex: include alloca.h before undef it
>   graph.c: limit linkage of internal variable
>   progress.c: silence cgcc suggestion about internal linkage
> 
>  compat/regex/regex.c                | 1 +
>  graph.c                             | 2 +-
>  progress.c                          | 2 +-
>  progress.h                          | 8 ++++++++
>  t/helper/test-parse-pathspec-file.c | 6 +++---
>  t/helper/test-progress.c            | 9 +--------
>  6 files changed, 15 insertions(+), 13 deletions(-)
> 

> It's happened that I tried to run make check on project that support autoconf.
> Git's Makefile told me to run `make sparse` instead.
>
> I /think/ if we have a rule in Makefile, we should adhere to it.
> I also fix another change in ds/blame-on-bloom, which I think it's worth to
> fix, see: <20200423133937.GA1984@danh.dev>

> Hi Danh,
> 
> On Thu, 23 Apr 2020, Đoàn Trần Công Danh wrote:
> 
> > It's happened that I tried to run make check on project that support autoconf.
> > Git's Makefile told me to run `make sparse` instead.
> >
> > I /think/ if we have a rule in Makefile, we should adhere to it.
> > I also fix another change in ds/blame-on-bloom, which I think it's worth to
> > fix, see: <20200423133937.GA1984@danh.dev>
> 
> FWIW there is still an Azure Pipeline building `sparse` for Ubuntu every
> two weeks: https://dev.azure.com/git/git/_build?definitionId=10&_a=summary
> 
> I created this Pipeline in order to support the proposed project at
> https://github.com/gitgitgadget/git/issues/345 which is: teach our CI
> builds to run `make sparse`.
> 
> Maybe it is time to tackle that?

> On 2020-04-25 15:13:53+0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> Hi Danh,
>>
>> On Thu, 23 Apr 2020, Đoàn Trần Công Danh wrote:
>>
>>> It's happened that I tried to run make check on project that support autoconf.
>>> Git's Makefile told me to run `make sparse` instead.
>>>
>>> I /think/ if we have a rule in Makefile, we should adhere to it.
>>> I also fix another change in ds/blame-on-bloom, which I think it's worth to
>>> fix, see: <20200423133937.GA1984@danh.dev>
>>
>> FWIW there is still an Azure Pipeline building `sparse` for Ubuntu every
>> two weeks: https://dev.azure.com/git/git/_build?definitionId=10&_a=summary
>>
>> I created this Pipeline in order to support the proposed project at
>> https://github.com/gitgitgadget/git/issues/345 which is: teach our CI
>> builds to run `make sparse`.
>>
>> Maybe it is time to tackle that?
> 
> I don't think it's ready, yet!

> 
> May be it's.
> 
> But we'll need
> 
> 	make sparse |
> 	grep -v -e 'plain integer as NULL pointer' \
> 		-e 'redeclared with different type'
> 
> The first one for:
> 
> 	struct foo val = { 0 };
> 
> to zero structure.
> 
> The second one for different in declaration and definition (which is
> fixed in sparse's master).
> 
> 

> On 26/04/2020 04:32, Danh Doan wrote:
> > On 2020-04-25 15:13:53+0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >> Hi Danh,
> >>
> >> On Thu, 23 Apr 2020, Đoàn Trần Công Danh wrote:
> >>
> >>> It's happened that I tried to run make check on project that support autoconf.
> >>> Git's Makefile told me to run `make sparse` instead.
> >>>
> >>> I /think/ if we have a rule in Makefile, we should adhere to it.
> >>> I also fix another change in ds/blame-on-bloom, which I think it's worth to
> >>> fix, see: <20200423133937.GA1984@danh.dev>
> >>
> >> FWIW there is still an Azure Pipeline building `sparse` for Ubuntu every
> >> two weeks: https://dev.azure.com/git/git/_build?definitionId=10&_a=summary
> >>
> >> I created this Pipeline in order to support the proposed project at
> >> https://github.com/gitgitgadget/git/issues/345 which is: teach our CI
> >> builds to run `make sparse`.
> >>
> >> Maybe it is time to tackle that?
> >
> > I don't think it's ready, yet!
>
> Heh, not too long ago, the 'master' and 'next' branches were 'sparse clean'.
> (on non NO_REGEX builds anyway, and even on those for some of us. ;-) )

>
> ATB,
> Ramsay Jones
>
> >
> > May be it's.
> >
> > But we'll need
> >
> > 	make sparse |
> > 	grep -v -e 'plain integer as NULL pointer' \
> > 		-e 'redeclared with different type'
> >
> > The first one for:
> >
> > 	struct foo val = { 0 };
> >
> > to zero structure.
> >
> > The second one for different in declaration and definition (which is
> > fixed in sparse's master).
> >
> >
>

> In Linux with musl libc, we have this inclusion chain:
> 
> compat/regex/regex.c:69
> `-> compat/regex/regex_internal.h
>    `-> /usr/include/stdlib.h
>       `-> /usr/include/features.h
>       `-> /usr/include/alloca.h
> 
> In that inclusion chain, `<features.h>` claims it's _BSD_SOURCE
> compatible when it's NOT asked to be either
> {_POSIX,_GNU,_XOPEN,_BSD}_SOURCE, or __STRICT_ANSI__.
> And, `<stdlib.h>` will include `<alloca.h>` to be compatible with
> software written for GNU and BSD. Thus, redefine `alloca` macro,
> which was defined before at compat/regex/regex.c:66.
> 
> Considering this is only compat code, we've taken from other project,
> it's not our business to decide which source should we adhere to.
> 
> Include `<stdlib.h>` early to prevent the redefinition of alloca.
> This also remove a potential warning about alloca not defined on:
> 	#undef alloca
> 
> Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
> 
> Notes:
>     There is another warning about un-match declaration and definition of
>     re_string_context_at.
>     
>     It's arguably, a bug in sparse itself.
>     Consider that there's a fix for it in their development version already,
>     and, we expect they'll fix the zero-ing pattern in 1/? before enable it again.

>     There're no point to change that pair of declaration/definition.
> 
>     This patch is technically Ramsay's work.
>     Since <alloca.h> is likely unportable, my patch will be likely unusable.
>     I took Ramsay's work and write the commit message instead.
> 
>     I would love to see Ramsay's SoB on reply to this patch.

> 
>  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
> 

> diff --git a/progress.h b/progress.h
> index 847338911f..f1913acf73 100644
> --- a/progress.h
> +++ b/progress.h
> @@ -3,6 +3,14 @@
>
>  struct progress;
>
> +#ifdef GIT_TEST_PROGRESS_ONLY
> +
> +extern int progress_testing;
> +extern uint64_t progress_test_ns;
> +void progress_test_force_update(void);
> +
> +#endif
> +
>  void display_throughput(struct progress *progress, uint64_t total);
>  void display_progress(struct progress *progress, uint64_t n);
>  struct progress *start_progress(const char *title, uint64_t total);

> 
> Change in v2 from v1:
> * = { 0 } is an idiom to zero the structure, it shouldn't be changed
> * change in [4/4] for prototype
> * ignore diffent function prototype
> 
> 
> Đoàn Trần Công Danh (4):
>   test-parse-pathspec-file.c: s/0/NULL/ for pointer type
>   compat/regex: move stdlib.h up in inclusion chain
>   graph.c: limit linkage of internal variable
>   progress.c: silence cgcc suggestion about internal linkage
> 
>  compat/regex/regex.c                | 1 +
>  compat/regex/regex_internal.h       | 1 -
>  graph.c                             | 2 +-
>  progress.c                          | 2 +-
>  progress.h                          | 8 ++++++++
>  t/helper/test-parse-pathspec-file.c | 6 +++---
>  t/helper/test-progress.c            | 9 +--------
>  7 files changed, 15 insertions(+), 14 deletions(-)
> 
> Range-diff against v2:
> 1:  c9a1812abf = 1:  c9a1812abf test-parse-pathspec-file.c: s/0/NULL/ for pointer type
> 2:  290ba923b5 < -:  ---------- compat/regex: include alloca.h before undef it
> -:  ---------- > 2:  8d18c53bc8 compat/regex: move stdlib.h up in inclusion chain
> 3:  39f8d85c2f = 3:  4e7580e1d1 graph.c: limit linkage of internal variable
> 4:  41eecf18ed = 4:  d66d9aa677 progress.c: silence cgcc suggestion about internal linkage
>