diff mbox series

[RFC,1/2] packfile API: add and use a pack_name_to_ext() utility function

Message ID RFC-patch-1.2-d8c3c03e90f-20220519T113538Z-avarab@gmail.com (mailing list archive)
State New
Headers show
Series Utility functions for duplicated pack(write) code | expand

Commit Message

Ævar Arnfjörð Bjarmason May 19, 2022, 11:42 a.m. UTC
Add and use a pack_name_to_ext() utility function for the copy/pasted
cases of creating a FOO.ext file given a string like FOO.pack.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 pack-bitmap.c   |  6 +-----
 pack-revindex.c |  5 +----
 packfile.c      | 14 ++++++++++----
 packfile.h      |  9 +++++++++
 4 files changed, 21 insertions(+), 13 deletions(-)

Comments

Junio C Hamano May 19, 2022, 3:40 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Add and use a pack_name_to_ext() utility function for the copy/pasted
> cases of creating a FOO.ext file given a string like FOO.pack.

I agree that "remove .pack extension and replace it with .foo
extention" is a common thing to do and it would be a welcome
simplification.

But pack_name_to_ext() sounds like taking pack-123456...9876.pack
and returning its extension (i.e. ".pack") that will become useful
when we introduce a drastically different naming convention and
start calling packfiles in newer format with different extensions
like ".pac4".

I wonder if this has easier-to-understand name that would be equally
(or slightly more) useful?

	char *replace_ext(const char *name, const char *src, const char *dst)
	{
		size_t len;

		if (!strip_suffix(name, src, &len))
			BUG("name '%s' does not end in suffix '%s'", name, src);
		return xstrfmt("%.*s.%s", (int)len, name, dst);
	}




> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  pack-bitmap.c   |  6 +-----
>  pack-revindex.c |  5 +----
>  packfile.c      | 14 ++++++++++----
>  packfile.h      |  9 +++++++++
>  4 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 97909d48da3..0c3770d038d 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -302,11 +302,7 @@ char *midx_bitmap_filename(struct multi_pack_index *midx)
>  
>  char *pack_bitmap_filename(struct packed_git *p)
>  {
> -	size_t len;
> -
> -	if (!strip_suffix(p->pack_name, ".pack", &len))
> -		BUG("pack_name does not end in .pack");
> -	return xstrfmt("%.*s.bitmap", (int)len, p->pack_name);
> +	return pack_name_to_ext(p->pack_name, "bitmap");
>  }
>  
>  static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
> diff --git a/pack-revindex.c b/pack-revindex.c
> index 08dc1601679..69dc5688796 100644
> --- a/pack-revindex.c
> +++ b/pack-revindex.c
> @@ -179,10 +179,7 @@ static int create_pack_revindex_in_memory(struct packed_git *p)
>  
>  static char *pack_revindex_filename(struct packed_git *p)
>  {
> -	size_t len;
> -	if (!strip_suffix(p->pack_name, ".pack", &len))
> -		BUG("pack_name does not end in .pack");
> -	return xstrfmt("%.*s.rev", (int)len, p->pack_name);
> +	return pack_name_to_ext(p->pack_name, "rev");
>  }
>  
>  #define RIDX_HEADER_SIZE (12)
> diff --git a/packfile.c b/packfile.c
> index 835b2d27164..bd6ad441bf5 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -191,15 +191,12 @@ int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
>  int open_pack_index(struct packed_git *p)
>  {
>  	char *idx_name;
> -	size_t len;
>  	int ret;
>  
>  	if (p->index_data)
>  		return 0;
>  
> -	if (!strip_suffix(p->pack_name, ".pack", &len))
> -		BUG("pack_name does not end in .pack");
> -	idx_name = xstrfmt("%.*s.idx", (int)len, p->pack_name);
> +	idx_name = pack_name_to_ext(p->pack_name, "idx");
>  	ret = check_packed_git_idx(idx_name, p);
>  	free(idx_name);
>  	return ret;
> @@ -2266,3 +2263,12 @@ int is_promisor_object(const struct object_id *oid)
>  	}
>  	return oidset_contains(&promisor_objects, oid);
>  }
> +
> +char *pack_name_to_ext(const char *pack_name, const char *ext)
> +{
> +	size_t len;
> +
> +	if (!strip_suffix(pack_name, ".pack", &len))
> +		BUG("pack_name does not end in .pack");
> +	return xstrfmt("%.*s.%s", (int)len, pack_name, ext);
> +}
> diff --git a/packfile.h b/packfile.h
> index a3f6723857b..6890c57ebdb 100644
> --- a/packfile.h
> +++ b/packfile.h
> @@ -195,4 +195,13 @@ int is_promisor_object(const struct object_id *oid);
>  int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
>  	     size_t idx_size, struct packed_git *p);
>  
> +/**
> + * Given a string like "foo.pack" and "ext" returns an xstrdup()'d
> + * "foo.ext" string. Used for creating e.g. PACK.{bitmap,rev,...}
> + * filenames from PACK.pack.
> + *
> + * Will BUG() if the expected string can't be created from the
> + * "pack_name" argument.
> + */
> +char *pack_name_to_ext(const char *pack_name, const char *ext);
>  #endif
diff mbox series

Patch

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 97909d48da3..0c3770d038d 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -302,11 +302,7 @@  char *midx_bitmap_filename(struct multi_pack_index *midx)
 
 char *pack_bitmap_filename(struct packed_git *p)
 {
-	size_t len;
-
-	if (!strip_suffix(p->pack_name, ".pack", &len))
-		BUG("pack_name does not end in .pack");
-	return xstrfmt("%.*s.bitmap", (int)len, p->pack_name);
+	return pack_name_to_ext(p->pack_name, "bitmap");
 }
 
 static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
diff --git a/pack-revindex.c b/pack-revindex.c
index 08dc1601679..69dc5688796 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -179,10 +179,7 @@  static int create_pack_revindex_in_memory(struct packed_git *p)
 
 static char *pack_revindex_filename(struct packed_git *p)
 {
-	size_t len;
-	if (!strip_suffix(p->pack_name, ".pack", &len))
-		BUG("pack_name does not end in .pack");
-	return xstrfmt("%.*s.rev", (int)len, p->pack_name);
+	return pack_name_to_ext(p->pack_name, "rev");
 }
 
 #define RIDX_HEADER_SIZE (12)
diff --git a/packfile.c b/packfile.c
index 835b2d27164..bd6ad441bf5 100644
--- a/packfile.c
+++ b/packfile.c
@@ -191,15 +191,12 @@  int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
 int open_pack_index(struct packed_git *p)
 {
 	char *idx_name;
-	size_t len;
 	int ret;
 
 	if (p->index_data)
 		return 0;
 
-	if (!strip_suffix(p->pack_name, ".pack", &len))
-		BUG("pack_name does not end in .pack");
-	idx_name = xstrfmt("%.*s.idx", (int)len, p->pack_name);
+	idx_name = pack_name_to_ext(p->pack_name, "idx");
 	ret = check_packed_git_idx(idx_name, p);
 	free(idx_name);
 	return ret;
@@ -2266,3 +2263,12 @@  int is_promisor_object(const struct object_id *oid)
 	}
 	return oidset_contains(&promisor_objects, oid);
 }
+
+char *pack_name_to_ext(const char *pack_name, const char *ext)
+{
+	size_t len;
+
+	if (!strip_suffix(pack_name, ".pack", &len))
+		BUG("pack_name does not end in .pack");
+	return xstrfmt("%.*s.%s", (int)len, pack_name, ext);
+}
diff --git a/packfile.h b/packfile.h
index a3f6723857b..6890c57ebdb 100644
--- a/packfile.h
+++ b/packfile.h
@@ -195,4 +195,13 @@  int is_promisor_object(const struct object_id *oid);
 int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
 	     size_t idx_size, struct packed_git *p);
 
+/**
+ * Given a string like "foo.pack" and "ext" returns an xstrdup()'d
+ * "foo.ext" string. Used for creating e.g. PACK.{bitmap,rev,...}
+ * filenames from PACK.pack.
+ *
+ * Will BUG() if the expected string can't be created from the
+ * "pack_name" argument.
+ */
+char *pack_name_to_ext(const char *pack_name, const char *ext);
 #endif