On Mon, Mar 8, 2021 at 7:07 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote:
>
> Refactor a couple of "switch" statements that previously relied on
> "entry.mode" to switch on "entry.object_type" instead.
>
> This is more obvious, and allows us to explicitly handle all the OBJ_*
> cases, not just have a wildcard "else". That doesn't matter for the
> behavior of this code, but for its readability and maintainability.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> list-objects.c | 20 ++++++++++++++------
> walker.c | 22 +++++++++++++---------
> 2 files changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/list-objects.c b/list-objects.c
> index e19589baa04..37434ba89d3 100644
> --- a/list-objects.c
> +++ b/list-objects.c
> @@ -111,6 +111,9 @@ static void process_tree_contents(struct traversal_context *ctx,
> init_tree_desc(&desc, tree->buffer, tree->size);
>
> while (tree_entry(&desc, &entry)) {
> + struct tree *t;
> + struct blob *b;
> +
> if (match != all_entries_interesting) {
> match = tree_entry_interesting(ctx->revs->repo->index,
> &entry, base, 0,
> @@ -121,8 +124,9 @@ static void process_tree_contents(struct traversal_context *ctx,
> continue;
> }
>
> - if (S_ISDIR(entry.mode)) {
> - struct tree *t = lookup_tree(ctx->revs->repo, &entry.oid);
> + switch (entry.object_type) {
> + case OBJ_TREE:
> + t = lookup_tree(ctx->revs->repo, &entry.oid);
> if (!t) {
> die(_("entry '%s' in tree %s has tree mode, "
> "but is not a tree"),
> @@ -130,12 +134,13 @@ static void process_tree_contents(struct traversal_context *ctx,
> }
> t->object.flags |= NOT_USER_GIVEN;
> process_tree(ctx, t, base, entry.path);
> - }
> - else if (S_ISGITLINK(entry.mode))
> + break;
> + case OBJ_COMMIT:
> process_gitlink(ctx, entry.oid.hash,
> base, entry.path);
> - else {
> - struct blob *b = lookup_blob(ctx->revs->repo, &entry.oid);
> + break;
> + case OBJ_BLOB:
> + b = lookup_blob(ctx->revs->repo, &entry.oid);
> if (!b) {
> die(_("entry '%s' in tree %s has blob mode, "
> "but is not a blob"),
> @@ -143,6 +148,9 @@ static void process_tree_contents(struct traversal_context *ctx,
> }
> b->object.flags |= NOT_USER_GIVEN;
> process_blob(ctx, b, base, entry.path);
> + break;
> + default:
> + BUG("unreachable");
> }
> }
> }
> diff --git a/walker.c b/walker.c
> index 4984bf8b3d6..7ba757244e6 100644
> --- a/walker.c
> +++ b/walker.c
> @@ -45,21 +45,25 @@ static int process_tree(struct walker *walker, struct tree *tree)
> init_tree_desc(&desc, tree->buffer, tree->size);
> while (tree_entry(&desc, &entry)) {
> struct object *obj = NULL;
> + struct tree *tree;
> + struct blob *blob;
>
> - /* submodule commits are not stored in the superproject */
> - if (S_ISGITLINK(entry.mode))
> + switch (entry.object_type) {
> + case OBJ_COMMIT:
> + /* submodule commits are not stored in the superproject */
> continue;
> - if (S_ISDIR(entry.mode)) {
> - struct tree *tree = lookup_tree(the_repository,
> - &entry.oid);
> + case OBJ_TREE:
> + tree = lookup_tree(the_repository, &entry.oid);
> if (tree)
> obj = &tree->object;
> - }
> - else {
> - struct blob *blob = lookup_blob(the_repository,
> - &entry.oid);
> + break;
> + case OBJ_BLOB:
> + blob = lookup_blob(the_repository, &entry.oid);
> if (blob)
> obj = &blob->object;
> + break;
> + default:
> + BUG("unreachable");
> }
> if (!obj || process(walker, obj))
> return -1;
> --
> 2.31.0.rc0.126.g04f22c5b82
This does make it nicer. :-)
