Re: [PATCH] fast-export: factor out print_oid()

Am 13.08.20 um 17:18 schrieb Taylor Blau:

On Thu, Aug 13, 2020 at 01:11:18PM +0200, René Scharfe wrote:

quoted

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 <redacted>
---
 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 <redacted>
Helped-by: Jeff King [off-list ref]
Signed-off-by: René Scharfe <redacted>
---
 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