| Message ID | 20210308150650.18626-30-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: > > Rename the "mode" variable passed to read_tree_fn_t callbacks to > "raw_mode". This variable comes to us from the tree-walk.h API. By > renaming this variable we can easily see where its downstream users > are in a subsequent commit where we'll sprinkle some canon_mode() > here. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > archive.c | 5 +++-- > builtin/log.c | 2 +- > builtin/ls-files.c | 11 ++++++----- > builtin/ls-tree.c | 6 +++--- > merge-recursive.c | 2 +- > 5 files changed, 14 insertions(+), 12 deletions(-) > > diff --git a/archive.c b/archive.c > index bc8f1c7546f..5b85aae8106 100644 > --- a/archive.c > +++ b/archive.c > @@ -232,10 +232,11 @@ static int write_directory(struct archiver_context *c) > > static int queue_or_write_archive_entry(const struct object_id *oid, > struct strbuf *base, const char *filename, > - enum object_type object_type, unsigned mode, > + enum object_type object_type, unsigned raw_mode, > void *context) > { > struct archiver_context *c = context; > + unsigned mode = raw_mode; > > while (c->bottom && > !(base->len >= c->bottom->len && > @@ -382,7 +383,7 @@ struct path_exists_context { > > static int reject_entry(const struct object_id *oid, struct strbuf *base, > const char *filename, > - enum object_type object_type, unsigned mode, > + enum object_type object_type, unsigned raw_mode, > void *context) > { > int ret = -1; > diff --git a/builtin/log.c b/builtin/log.c > index 19a916221d5..c3ef1b3e22d 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -599,7 +599,7 @@ static int show_tag_object(const struct object_id *oid, struct rev_info *rev) > > static int show_tree_object(const struct object_id *oid, > struct strbuf *base, const char *pathname, > - enum object_type object_type, unsigned mode, > + enum object_type object_type, unsigned raw_mode, This was surprising to me and had me worried, but it looks like this function doesn't even use mode or raw_mode. > void *context) > { > FILE *file = context; > diff --git a/builtin/ls-files.c b/builtin/ls-files.c > index f38df439410..391e6a9f141 100644 > --- a/builtin/ls-files.c > +++ b/builtin/ls-files.c > @@ -425,10 +425,11 @@ static int read_one_entry_opt(struct index_state *istate, > const struct object_id *oid, > struct strbuf *base, > const char *pathname, > - unsigned mode, int opt) > + unsigned raw_mode, int opt) > { > int len; > struct cache_entry *ce; > + unsigned mode = raw_mode; I was about to comment, but checked out the code in question and it looks like you've modified further. So perhaps I should wait until the last patch in the series. > > if (S_ISDIR(mode)) > return READ_TREE_RECURSIVE; > @@ -447,12 +448,12 @@ static int read_one_entry_opt(struct index_state *istate, > > static int read_one_entry(const struct object_id *oid, struct strbuf *base, > const char *pathname, > - enum object_type object_type, unsigned mode, > + enum object_type object_type, unsigned raw_mode, > void *context) > { > struct index_state *istate = context; > return read_one_entry_opt(istate, oid, base, pathname, > - mode, > + raw_mode, > ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK); > } > > @@ -462,12 +463,12 @@ static int read_one_entry(const struct object_id *oid, struct strbuf *base, > */ > static int read_one_entry_quick(const struct object_id *oid, struct strbuf *base, > const char *pathname, > - enum object_type object_type, unsigned mode, > + enum object_type object_type, unsigned raw_mode, > void *context) > { > struct index_state *istate = context; > return read_one_entry_opt(istate, oid, base, pathname, > - mode, > + raw_mode, > ADD_CACHE_JUST_APPEND); > } > > diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c > index c6ec3ca751e..3f84603d391 100644 > --- a/builtin/ls-tree.c > +++ b/builtin/ls-tree.c > @@ -63,7 +63,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname) > > static int show_tree(const struct object_id *oid, struct strbuf *base, > const char *pathname, > - enum object_type object_type, unsigned mode, > + enum object_type object_type, unsigned raw_mode, > void *context) > { > int retval = 0; > @@ -103,11 +103,11 @@ static int show_tree(const struct object_id *oid, struct strbuf *base, > "%"PRIuMAX, (uintmax_t)size); > } else > xsnprintf(size_text, sizeof(size_text), "-"); > - printf("%06o %s %s %7s\t", mode, type, > + printf("%06o %s %s %7s\t", raw_mode, type, > find_unique_abbrev(oid, abbrev), > size_text); > } else > - printf("%06o %s %s\t", mode, type, > + printf("%06o %s %s\t", raw_mode, type, > find_unique_abbrev(oid, abbrev)); This looks like a behavioral change. It might be desirable, but shouldn't it be called out and documented with a testcase? (Or is there no change yet because the raw_mode isn't actually yet raw?) > } > baselen = base->len; > diff --git a/merge-recursive.c b/merge-recursive.c > index b26d9d418f9..30fbe72ca06 100644 > --- a/merge-recursive.c > +++ b/merge-recursive.c > @@ -453,7 +453,7 @@ static void unpack_trees_finish(struct merge_options *opt) > > static int save_files_dirs(const struct object_id *oid, > struct strbuf *base, const char *path, > - enum object_type object_type, unsigned int mode, > + enum object_type object_type, unsigned int raw_mode, > void *context) > { > struct path_hashmap_entry *entry; Oh, man, merge-recursive.c gets modes from the index, from the diff machinery, and from its own direct calls (get_tree_entry*() calls). And might have comparisons in all kinds of places. I'd be _very_ leery, much more so than with merge-ort.c, of having it deal with raw modes.
diff --git a/archive.c b/archive.c index bc8f1c7546f..5b85aae8106 100644 --- a/archive.c +++ b/archive.c @@ -232,10 +232,11 @@ static int write_directory(struct archiver_context *c) static int queue_or_write_archive_entry(const struct object_id *oid, struct strbuf *base, const char *filename, - enum object_type object_type, unsigned mode, + enum object_type object_type, unsigned raw_mode, void *context) { struct archiver_context *c = context; + unsigned mode = raw_mode; while (c->bottom && !(base->len >= c->bottom->len && @@ -382,7 +383,7 @@ struct path_exists_context { static int reject_entry(const struct object_id *oid, struct strbuf *base, const char *filename, - enum object_type object_type, unsigned mode, + enum object_type object_type, unsigned raw_mode, void *context) { int ret = -1; diff --git a/builtin/log.c b/builtin/log.c index 19a916221d5..c3ef1b3e22d 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -599,7 +599,7 @@ static int show_tag_object(const struct object_id *oid, struct rev_info *rev) static int show_tree_object(const struct object_id *oid, struct strbuf *base, const char *pathname, - enum object_type object_type, unsigned mode, + enum object_type object_type, unsigned raw_mode, void *context) { FILE *file = context; diff --git a/builtin/ls-files.c b/builtin/ls-files.c index f38df439410..391e6a9f141 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -425,10 +425,11 @@ static int read_one_entry_opt(struct index_state *istate, const struct object_id *oid, struct strbuf *base, const char *pathname, - unsigned mode, int opt) + unsigned raw_mode, int opt) { int len; struct cache_entry *ce; + unsigned mode = raw_mode; if (S_ISDIR(mode)) return READ_TREE_RECURSIVE; @@ -447,12 +448,12 @@ static int read_one_entry_opt(struct index_state *istate, static int read_one_entry(const struct object_id *oid, struct strbuf *base, const char *pathname, - enum object_type object_type, unsigned mode, + enum object_type object_type, unsigned raw_mode, void *context) { struct index_state *istate = context; return read_one_entry_opt(istate, oid, base, pathname, - mode, + raw_mode, ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK); } @@ -462,12 +463,12 @@ static int read_one_entry(const struct object_id *oid, struct strbuf *base, */ static int read_one_entry_quick(const struct object_id *oid, struct strbuf *base, const char *pathname, - enum object_type object_type, unsigned mode, + enum object_type object_type, unsigned raw_mode, void *context) { struct index_state *istate = context; return read_one_entry_opt(istate, oid, base, pathname, - mode, + raw_mode, ADD_CACHE_JUST_APPEND); } diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index c6ec3ca751e..3f84603d391 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -63,7 +63,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname) static int show_tree(const struct object_id *oid, struct strbuf *base, const char *pathname, - enum object_type object_type, unsigned mode, + enum object_type object_type, unsigned raw_mode, void *context) { int retval = 0; @@ -103,11 +103,11 @@ static int show_tree(const struct object_id *oid, struct strbuf *base, "%"PRIuMAX, (uintmax_t)size); } else xsnprintf(size_text, sizeof(size_text), "-"); - printf("%06o %s %s %7s\t", mode, type, + printf("%06o %s %s %7s\t", raw_mode, type, find_unique_abbrev(oid, abbrev), size_text); } else - printf("%06o %s %s\t", mode, type, + printf("%06o %s %s\t", raw_mode, type, find_unique_abbrev(oid, abbrev)); } baselen = base->len; diff --git a/merge-recursive.c b/merge-recursive.c index b26d9d418f9..30fbe72ca06 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -453,7 +453,7 @@ static void unpack_trees_finish(struct merge_options *opt) static int save_files_dirs(const struct object_id *oid, struct strbuf *base, const char *path, - enum object_type object_type, unsigned int mode, + enum object_type object_type, unsigned int raw_mode, void *context) { struct path_hashmap_entry *entry;
Rename the "mode" variable passed to read_tree_fn_t callbacks to "raw_mode". This variable comes to us from the tree-walk.h API. By renaming this variable we can easily see where its downstream users are in a subsequent commit where we'll sprinkle some canon_mode() here. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- archive.c | 5 +++-- builtin/log.c | 2 +- builtin/ls-files.c | 11 ++++++----- builtin/ls-tree.c | 6 +++--- merge-recursive.c | 2 +- 5 files changed, 14 insertions(+), 12 deletions(-)