On Mon, Mar 8, 2021 at 7:07 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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
>
