| Message ID | 20210308150650.18626-14-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Move the read_tree() function to its only user | expand |
On Mon, Mar 8, 2021 at 7:07 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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@gmail.com> > --- > 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. :-)
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;
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@gmail.com> --- list-objects.c | 20 ++++++++++++++------ walker.c | 22 +++++++++++++--------- 2 files changed, 27 insertions(+), 15 deletions(-)