Am 13.08.20 um 17:18 schrieb Taylor Blau: > On Thu, Aug 13, 2020 at 01:11:18PM +0200, René Scharfe wrote: >> Simplify the output code by splitting it up and reducing duplication. >> Reuse the logic for printing object IDs -- anonymized if needed -- by >> moving it to its own function, print_oid(). >> >> Signed-off-by: René Scharfe <l.s.r@xxxxxx> >> --- >> builtin/fast-export.c | 26 +++++++++++++++----------- >> 1 file changed, 15 insertions(+), 11 deletions(-) >> >> diff --git a/builtin/fast-export.c b/builtin/fast-export.c >> index 9f37895d4cf..49bb50634ab 100644 >> --- a/builtin/fast-export.c >> +++ b/builtin/fast-export.c >> @@ -420,6 +420,14 @@ static const char *anonymize_oid(const char *oid_hex) >> return anonymize_str(&objs, generate_fake_oid, oid_hex, len, NULL); >> } >> >> +static void print_oid(const struct object_id *oid, int anonymize) >> +{ >> + const char *oid_hex = oid_to_hex(oid); >> + if (anonymize) >> + oid_hex = anonymize_oid(oid_hex); >> + fputs(oid_hex, stdout); >> +} >> + > > The fact that this calls fputs makes this patch (in my own opinion) > noisier than it needs to be. This is because of all of the factoring out > of the other printfs. I'd expect that this looks something more like: > > - anonymize ? > - anonymize_oid(oid_to_hex(&spec->oid)) : > - oid_to_hex(&spec->oid)); > + anonymize_oid(anonymize, &spec->oid)); > > without moving around all of the other printf code. Moving that part to anonymize_oid() would reduce the line count while still getting rid of the duplication. But the function would need a new name. -- >8 -- Subject: [PATCH v2] fast-export: deduplicate anonymization handling Move the code for converting an object_id to a hexadecimal string and for handling of the default (not anonymizing) case from its callers to anonymize_oid() and consequently rename it to anonymize_oid_if_needed(). This reduces code duplication. Suggested-by: Taylor Blau <me@xxxxxxxxxxxx> Helped-by: Jeff King <peff@xxxxxxxx> Signed-off-by: René Scharfe <l.s.r@xxxxxx> --- builtin/fast-export.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index 9f37895d4cf..fcc3208727f 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -413,10 +413,13 @@ static char *generate_fake_oid(void *data) return hash_to_hex_algop_r(hex, out, the_hash_algo); } -static const char *anonymize_oid(const char *oid_hex) +static const char *anonymize_oid_if_needed(const struct object_id *oid) { static struct hashmap objs; + const char *oid_hex = oid_to_hex(oid); size_t len = strlen(oid_hex); + if (!anonymize) + return oid_hex; return anonymize_str(&objs, generate_fake_oid, oid_hex, len, NULL); } @@ -476,9 +479,7 @@ static void show_filemodify(struct diff_queue_struct *q, */ if (no_data || S_ISGITLINK(spec->mode)) printf("M %06o %s ", spec->mode, - anonymize ? - anonymize_oid(oid_to_hex(&spec->oid)) : - oid_to_hex(&spec->oid)); + anonymize_oid_if_needed(&spec->oid)); else { struct object *object = lookup_object(the_repository, &spec->oid); @@ -726,10 +727,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev, if (mark) printf(":%d\n", mark); else - printf("%s\n", - anonymize ? - anonymize_oid(oid_to_hex(&obj->oid)) : - oid_to_hex(&obj->oid)); + printf("%s\n", anonymize_oid_if_needed(&obj->oid)); i++; } -- 2.28.0