diff mbox series

[26/30] tree-walk.h API: add a tree_entry_extract_all() function

Message ID 20210308150650.18626-27-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series Move the read_tree() function to its only user | expand

Commit Message

Ævar Arnfjörð Bjarmason March 8, 2021, 3:06 p.m. UTC
Add a tree_entry_extract_all() sibling function to the existing
tree_entry_extract_mode().

Having the OBJ_{BLOB,TREE,COMMIT} when you have the "mode" is strictly
speaking redundant, but hopefully makes it easier to read the
code. We'll now see which parts of the code are checking the types,
v.s. those that care about the mode specifically.

Only the first use of tree_entry_extract_mode() in emit_path() is
converted here, the other branch will use a new
get_tree_entry_mode_type() introduced in a subsequent commit.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/update-index.c |  6 ++++--
 tree-diff.c            |  5 +++--
 tree-walk.c            |  3 ++-
 tree-walk.h            | 12 ++++++++++++
 4 files changed, 21 insertions(+), 5 deletions(-)

Comments

Elijah Newren March 9, 2021, 6:30 p.m. UTC | #1
On Mon, Mar 8, 2021 at 7:07 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>
> Add a tree_entry_extract_all() sibling function to the existing
> tree_entry_extract_mode().
>
> Having the OBJ_{BLOB,TREE,COMMIT} when you have the "mode" is strictly
> speaking redundant, but hopefully makes it easier to read the
> code. We'll now see which parts of the code are checking the types,
> v.s. those that care about the mode specifically.
>
> Only the first use of tree_entry_extract_mode() in emit_path() is
> converted here, the other branch will use a new
> get_tree_entry_mode_type() introduced in a subsequent commit.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/update-index.c |  6 ++++--
>  tree-diff.c            |  5 +++--
>  tree-walk.c            |  3 ++-
>  tree-walk.h            | 12 ++++++++++++
>  4 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 070510d6a88..b489a876392 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -599,16 +599,18 @@ static struct cache_entry *read_one_ent(const char *which,
>                                         struct object_id *ent, const char *path,
>                                         int namelen, int stage)
>  {
> +       enum object_type object_type;
>         unsigned short mode;
>         struct object_id oid;
>         struct cache_entry *ce;
>
> -       if (get_tree_entry_mode(the_repository, ent, path, &oid, &mode)) {
> +       if (get_tree_entry_all(the_repository, ent, path, &oid,
> +                              &mode, &object_type)) {
>                 if (which)
>                         error("%s: not in %s branch.", path, which);
>                 return NULL;
>         }
> -       if (mode == S_IFDIR) {
> +       if (object_type == OBJ_TREE) {
>                 if (which)
>                         error("%s: not a blob in %s branch.", path, which);
>                 return NULL;
> diff --git a/tree-diff.c b/tree-diff.c
> index b37348b7908..b25095c1164 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -195,10 +195,11 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
>         assert(t || tp);
>
>         if (t) {
> +               enum object_type object_type;
>                 /* path present in resulting tree */
> -               oid = tree_entry_extract_mode(t, &path, &mode);
> +               oid = tree_entry_extract_all(t, &path, &mode, &object_type);
>                 pathlen = tree_entry_len(&t->entry);
> -               isdir = S_ISDIR(mode);
> +               isdir = object_type == OBJ_TREE;

Not worth a reroll, but parenthesis around the right-hand side would
make it a bit faster to parse and understand.

>         } else {
>                 /*
>                  * a path was removed - take path from imin parent. Also take
> diff --git a/tree-walk.c b/tree-walk.c
> index e613f273767..12e0ed4e250 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -570,7 +570,8 @@ static int find_tree_entry(struct repository *r, struct tree_desc *t,
>                 struct object_id oid;
>                 int entrylen, cmp;
>
> -               oidcpy(&oid, tree_entry_extract_mode(t, &entry, mode));
> +               oidcpy(&oid, tree_entry_extract_all(t, &entry, mode, object_type));
> +
>                 entrylen = tree_entry_len(&t->entry);
>                 update_tree_entry(t);
>                 if (entrylen > namelen)
> diff --git a/tree-walk.h b/tree-walk.h
> index 892e77eda23..06ad40ab2f1 100644
> --- a/tree-walk.h
> +++ b/tree-walk.h
> @@ -47,6 +47,7 @@ struct tree_desc {
>   * appropriate variable to fill in (NULL won't do!):
>   *
>   * tree_entry_extract_mode(): const char *path, unsigned int mode
> + * tree_entry_extract_all(): const char *path, unsigned int mode, enum object_type
>   */
>  static inline const struct object_id *tree_entry_extract_mode(struct tree_desc *desc,
>                                                               const char **pathp,
> @@ -57,6 +58,17 @@ static inline const struct object_id *tree_entry_extract_mode(struct tree_desc *
>         return &desc->entry.oid;
>  }
>
> +static inline const struct object_id *tree_entry_extract_all(struct tree_desc *desc,
> +                                                            const char **pathp,
> +                                                            unsigned short *modep,
> +                                                            enum object_type *object_typep)
> +{
> +       *pathp = desc->entry.path;
> +       *modep = desc->entry.mode;
> +       *object_typep = desc->entry.object_type;
> +       return &desc->entry.oid;
> +}
> +
>  /**
>   * Calculate the length of a tree entry's pathname. This utilizes the
>   * memory structure of a tree entry to avoid the overhead of using a
> --
> 2.31.0.rc0.126.g04f22c5b82
>
diff mbox series

Patch

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 070510d6a88..b489a876392 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -599,16 +599,18 @@  static struct cache_entry *read_one_ent(const char *which,
 					struct object_id *ent, const char *path,
 					int namelen, int stage)
 {
+	enum object_type object_type;
 	unsigned short mode;
 	struct object_id oid;
 	struct cache_entry *ce;
 
-	if (get_tree_entry_mode(the_repository, ent, path, &oid, &mode)) {
+	if (get_tree_entry_all(the_repository, ent, path, &oid,
+			       &mode, &object_type)) {
 		if (which)
 			error("%s: not in %s branch.", path, which);
 		return NULL;
 	}
-	if (mode == S_IFDIR) {
+	if (object_type == OBJ_TREE) {
 		if (which)
 			error("%s: not a blob in %s branch.", path, which);
 		return NULL;
diff --git a/tree-diff.c b/tree-diff.c
index b37348b7908..b25095c1164 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -195,10 +195,11 @@  static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 	assert(t || tp);
 
 	if (t) {
+		enum object_type object_type;
 		/* path present in resulting tree */
-		oid = tree_entry_extract_mode(t, &path, &mode);
+		oid = tree_entry_extract_all(t, &path, &mode, &object_type);
 		pathlen = tree_entry_len(&t->entry);
-		isdir = S_ISDIR(mode);
+		isdir = object_type == OBJ_TREE;
 	} else {
 		/*
 		 * a path was removed - take path from imin parent. Also take
diff --git a/tree-walk.c b/tree-walk.c
index e613f273767..12e0ed4e250 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -570,7 +570,8 @@  static int find_tree_entry(struct repository *r, struct tree_desc *t,
 		struct object_id oid;
 		int entrylen, cmp;
 
-		oidcpy(&oid, tree_entry_extract_mode(t, &entry, mode));
+		oidcpy(&oid, tree_entry_extract_all(t, &entry, mode, object_type));
+
 		entrylen = tree_entry_len(&t->entry);
 		update_tree_entry(t);
 		if (entrylen > namelen)
diff --git a/tree-walk.h b/tree-walk.h
index 892e77eda23..06ad40ab2f1 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -47,6 +47,7 @@  struct tree_desc {
  * appropriate variable to fill in (NULL won't do!):
  *
  * tree_entry_extract_mode(): const char *path, unsigned int mode
+ * tree_entry_extract_all(): const char *path, unsigned int mode, enum object_type
  */
 static inline const struct object_id *tree_entry_extract_mode(struct tree_desc *desc,
 							      const char **pathp,
@@ -57,6 +58,17 @@  static inline const struct object_id *tree_entry_extract_mode(struct tree_desc *
 	return &desc->entry.oid;
 }
 
+static inline const struct object_id *tree_entry_extract_all(struct tree_desc *desc,
+							     const char **pathp,
+							     unsigned short *modep,
+							     enum object_type *object_typep)
+{
+	*pathp = desc->entry.path;
+	*modep = desc->entry.mode;
+	*object_typep = desc->entry.object_type;
+	return &desc->entry.oid;
+}
+
 /**
  * Calculate the length of a tree entry's pathname. This utilizes the
  * memory structure of a tree entry to avoid the overhead of using a