| Message ID | 1908832c-8dd0-377e-917b-acb33b00273c@ramsayjones.plus.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | commit-graph: fix sparse 'NULL pointer' warning | expand |
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 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. > 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);
On 25/03/2019 12:02, Ævar Arnfjörð Bjarmason wrote: > > 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 ... ;-) > >> 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
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);
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(-)