Thread (193 messages) flat view 193 messages, 3 authors, 2021-03-21

Re: [PATCH 13/30] tree-walk.h users: refactor chained "mode" if/else into switch

From: Elijah Newren <hidden>
Date: 2021-03-09 17:12:29

On Mon, Mar 8, 2021 at 7:07 AM Ævar Arnfjörð Bjarmason [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
 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.  :-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help