threads / patch / 50808

patchcommit-graph: fix sparse 'NULL pointer' warning

Subject: [PATCH] commit-graph: fix sparse 'NULL pointer' warning

## tl;dr

3 messages between Mar 23, 2019 and Mar 25, 2019. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Ramsay Jones· Mar 23, 2019, 01:19 UTC · lore

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi Ævar,

If you need to re-roll your 'ab/commit-graph-fixes' branch, could you please squash this into the relevant patch (commit aeb244adbc ("commit-graph: don't early exit(1) on e.g. \"git status\"", 2019-02-21).

This same commit (aeb244adbc) also removes the last call, outside of the commit-graph.c file, to the function load_commit_graph_one(). So, this function is now a file-local symbol and could be marked 'static'.

Also, the function verify_commit_graph_lite(), introduced in commit d8acf37ff7 ("commit-graph: fix segfault on e.g. \"git status\"", 2019-02-21), currently has no external callers. This function is also a file-local symbol and could be marked 'static', unless you have plans for future external calls?

Thanks!

ATB, Ramsay Jones

 commit-graph.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to commit-graph.c +1 −1

diff --git a/commit-graph.c b/commit-graph.c
index 4696ee0036..680c6f5714 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -100,7 +100,7 @@ struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st)
 	if (graph_size < GRAPH_MIN_SIZE) {
 		close(fd);
 		error(_("commit-graph file is too small"));
-		return 0;
+		return NULL;
 	}
 	graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
 	ret = parse_commit_graph(graph_map, fd, graph_size);

-- 
2.21.0

Ævar Arnfjörð Bjarmason· Mar 25, 2019, 12:02 UTC · re: Ramsay Jones · lore

Re: [PATCH] commit-graph: fix sparse 'NULL pointer' warning

On Sat, Mar 23 2019, Ramsay Jones wrote:

Show 8 quoted lines

> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
>
> Hi Ævar,
>
> If you need to re-roll your 'ab/commit-graph-fixes' branch, could you
> please squash this into the relevant patch (commit aeb244adbc
> ("commit-graph: don't early exit(1) on e.g. \"git status\"", 2019-02-21).

Thanks. Will squash & re-submit. It's still just in "pu". Is there a compiler that warns about it? Didn't on clang/gcc, but then again it's just-as-valid-C & just a style issue, so compilers don't care...

Show 9 quoted lines

> This same commit (aeb244adbc) also removes the last call, outside of the
> commit-graph.c file, to the function load_commit_graph_one(). So, this
> function is now a file-local symbol and could be marked 'static'.
>
> Also, the function verify_commit_graph_lite(), introduced in commit
> d8acf37ff7 ("commit-graph: fix segfault on e.g. \"git status\"",
> 2019-02-21), currently has no external callers. This function is also
> a file-local symbol and could be marked 'static', unless you have plans
> for future external calls?

Fixing these too. Just missed them. Thanks.

Show 16 quoted lines

>  commit-graph.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index 4696ee0036..680c6f5714 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -100,7 +100,7 @@ struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st)
>  	if (graph_size < GRAPH_MIN_SIZE) {
>  		close(fd);
>  		error(_("commit-graph file is too small"));
> -		return 0;
> +		return NULL;
>  	}
>  	graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
>  	ret = parse_commit_graph(graph_map, fd, graph_size);

Ramsay Jones· Mar 25, 2019, 17:03 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] commit-graph: fix sparse 'NULL pointer' warning

On 25/03/2019 12:02, Ævar Arnfjörð Bjarmason wrote:

Show 15 quoted lines

> 
> On Sat, Mar 23 2019, Ramsay Jones wrote:
> 
>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
>> ---
>>
>> Hi Ævar,
>>
>> If you need to re-roll your 'ab/commit-graph-fixes' branch, could you
>> please squash this into the relevant patch (commit aeb244adbc
>> ("commit-graph: don't early exit(1) on e.g. \"git status\"", 2019-02-21).
> 
> Thanks. Will squash & re-submit. It's still just in "pu". Is there a
> compiler that warns about it? Didn't on clang/gcc, but then again it's
> just-as-valid-C & just a style issue, so compilers don't care...

This is a 'sparse' warning (see subject line). :-D

I run the 'sparse' Makefile target on each branch and save to a file, sp-out, nsp-out and psp-out, then diff from branch to branch. So, currently, the diff between the 'next' and 'pu' branches look like:

  $ diff nsp-out psp-out
  25a26
  > commit-graph.c:103:24: warning: Using plain integer as NULL pointer
  123a125
  >     SP promisor-remote.c
  188a191
  > upload-pack.c:1167:56: warning: Using plain integer as NULL pointer
  289d291
  <     SP builtin/rebase--interactive.c
  $

Alternatively, I can simply run 'sparse' over a single file:

  $ make commit-graph.sp
      SP commit-graph.c
  commit-graph.c:103:24: warning: Using plain integer as NULL pointer
  $

Of course, you need to have 'sparse' installed ... ;-)

Show 12 quoted lines

> 
>> This same commit (aeb244adbc) also removes the last call, outside of the
>> commit-graph.c file, to the function load_commit_graph_one(). So, this
>> function is now a file-local symbol and could be marked 'static'.
>>
>> Also, the function verify_commit_graph_lite(), introduced in commit
>> d8acf37ff7 ("commit-graph: fix segfault on e.g. \"git status\"",
>> 2019-02-21), currently has no external callers. This function is also
>> a file-local symbol and could be marked 'static', unless you have plans
>> for future external calls?
> 
> Fixing these too. Just missed them. Thanks.

And these resulted from running my static-check.pl script.

ATB, Ramsay Jones