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

Re: [PATCH 29/30] tree.h API users: rename read_tree_fn_t's "mode" to "raw_mode"

From: Elijah Newren <hidden>
Date: 2021-03-09 19:03:39

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