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
