On Thu, Jul 09, 2020 at 10:01:08PM -0400, Derrick Stolee wrote: > On 7/9/2020 9:42 PM, Emily Shaffer wrote: > > display_progress() and stop_progress() are tolerant to NULL inputs. > > Remove some places where redundant checks are performed before these > > NULL-tolerant functions are called. > > > > Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> > > --- > > builtin/commit-graph.c | 3 +-- > > commit-graph.c | 3 +-- > > read-cache.c | 9 +++------ > > 3 files changed, 5 insertions(+), 10 deletions(-) > > > > diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c > > index ae4dc2dfcd..7964de3126 100644 > > --- a/builtin/commit-graph.c > > +++ b/builtin/commit-graph.c > > @@ -263,8 +263,7 @@ static int graph_write(int argc, const char **argv) > > cleanup: > > string_list_clear(&pack_indexes, 0); > > strbuf_release(&buf); > > - if (progress) > > - stop_progress(&progress); > > + stop_progress(&progress); > > return result; > > } > > > > diff --git a/commit-graph.c b/commit-graph.c > > index b9a784fece..30d9a75932 100644 > > --- a/commit-graph.c > > +++ b/commit-graph.c > > @@ -1424,8 +1424,7 @@ int write_commit_graph_reachable(struct object_directory *odb, > > flags, split_opts); > > > > oidset_clear(&commits); > > - if (data.progress) > > - stop_progress(&data.progress); > > + stop_progress(&data.progress); > > return result; > > } > > > > diff --git a/read-cache.c b/read-cache.c > > index 2ddc422dbd..feb7abe37a 100644 > > --- a/read-cache.c > > +++ b/read-cache.c > > @@ -1581,8 +1581,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, > > new_entry = refresh_cache_ent(istate, ce, options, &cache_errno, &changed); > > if (new_entry == ce) > > continue; > > - if (progress) > > - display_progress(progress, i); > > + display_progress(progress, i); > > if (!new_entry) { > > const char *fmt; > > > > @@ -1614,10 +1613,8 @@ int refresh_index(struct index_state *istate, unsigned int flags, > > > > replace_index_entry(istate, i, new_entry); > > } > > - if (progress) { > > - display_progress(progress, istate->cache_nr); > > - stop_progress(&progress); > > - } > > + display_progress(progress, istate->cache_nr); > > + stop_progress(&progress); > > trace_performance_leave("refresh index"); > > return has_errors; > > Looks obviously correct to me. To me too, although note that this will generate merge conflicts with Szeder's patch from earlier today[1]. Unfortunately, the conflicts are a little deeper than "we both removed an unnecessary 'if' statement", since Szeder's patch moves the 'stop_progress()' call earlier to avoid a bug that I introduced. This is just something for Junio to look out for while queuing, otherwise I think just removing the hunk in builtin/commit-graph.c would be fine, too. > Thanks, > -Stolee Thanks, Taylor [1]: https://lore.kernel.org/git/xmqq5zawy4x5.fsf@xxxxxxxxxxxxxxxxxxxxxx/T/#mbd2d5236a7fecf1287be75ffa396f3776fc9b891