Re: [PATCH v3 07/10] progress.c: add temporary variable from progress struct
flat view
From: Taylor Blau <hidden>
Date: 2021-10-22 22:37:34
On Thu, Oct 14, 2021 at 12:28:23AM +0200, Ævar Arnfjörð Bjarmason wrote:
quoted hunk ↗ jump to hunk
Add a temporary "progress" variable for the dereferenced p_progress pointer to a "struct progress *". Before 98a13647408 (trace2: log progress time and throughput, 2020-05-12) we didn't dereference "p_progress" in this function, now that we do it's easier to read the code if we work with a "progress" struct pointer like everywhere else, instead of a pointer to a pointer. Signed-off-by: Ævar Arnfjörð Bjarmason <redacted> --- progress.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)diff --git a/progress.c b/progress.c index 7fcc513717a..b9369e9a264 100644 --- a/progress.c +++ b/progress.c@@ -329,15 +329,16 @@ void stop_progress(struct progress **p_progress) finish_if_sparse(*p_progress);
This spot also wants to dereference p_progress, but doesn't access any of the fields by dereferencing the second pointer. So this could certainly have passed `progress` (had you scoped its declaration to the whole function). I don't think it hurts readability, and I definitely don't have a strong opinion about it. Just an idle thought while reading this patch...
quoted hunk ↗ jump to hunk
if (*p_progress) { + struct progress *progress = *p_progress; trace2_data_intmax("progress", the_repository, "total_objects", (*p_progress)->total); if ((*p_progress)->throughput) trace2_data_intmax("progress", the_repository, "total_bytes", - (*p_progress)->throughput->curr_total); + progress->throughput->curr_total); - trace2_region_leave("progress", (*p_progress)->title, the_repository); + trace2_region_leave("progress", progress->title, the_repository);
This all looks much better to me. Thanks. Thanks, Taylor