]> git.ipfire.org Git - thirdparty/git.git/commitdiff
has_dir_name(): make code more obvious
authorJohannes Schindelin <johannes.schindelin@gmx.de>
Thu, 15 May 2025 13:11:43 +0000 (13:11 +0000)
committerJunio C Hamano <gitster@pobox.com>
Thu, 15 May 2025 20:46:46 +0000 (13:46 -0700)
One thing that might be non-obvious to readers (or to analyzers like
CodeQL) is that the function essentially does nothing when the Git index
is empty, and in particular that it does not look at the value of
`len_eq_last` (which would be uninitialized at that point).

Let's make this much easier to understand, by returning early if the Git
index is empty, and by avoiding empty `else` blocks.

This commit changes indentation and is hence best viewed using
`--ignore-space-change`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
read-cache.c

index 73f83a7e7a113ed8ef56a18a5b62a45908ceaa2a..c0bb760ad473efdf91641e4d28954b3e17d4e9df 100644 (file)
@@ -1117,48 +1117,19 @@ static int has_dir_name(struct index_state *istate,
         *
         * Compare the entry's full path with the last path in the index.
         */
-       if (istate->cache_nr > 0) {
-               cmp_last = strcmp_offset(name,
-                       istate->cache[istate->cache_nr - 1]->name,
-                       &len_eq_last);
-               if (cmp_last > 0) {
-                       if (name[len_eq_last] != '/') {
-                               /*
-                                * The entry sorts AFTER the last one in the
-                                * index.
-                                *
-                                * If there were a conflict with "file", then our
-                                * name would start with "file/" and the last index
-                                * entry would start with "file" but not "file/".
-                                *
-                                * The next character after common prefix is
-                                * not '/', so there can be no conflict.
-                                */
-                               return retval;
-                       } else {
-                               /*
-                                * The entry sorts AFTER the last one in the
-                                * index, and the next character after common
-                                * prefix is '/'.
-                                *
-                                * Either the last index entry is a file in
-                                * conflict with this entry, or it has a name
-                                * which sorts between this entry and the
-                                * potential conflicting file.
-                                *
-                                * In both cases, we fall through to the loop
-                                * below and let the regular search code handle it.
-                                */
-                       }
-               } else if (cmp_last == 0) {
-                       /*
-                        * The entry exactly matches the last one in the
-                        * index, but because of multiple stage and CE_REMOVE
-                        * items, we fall through and let the regular search
-                        * code handle it.
-                        */
-               }
-       }
+       if (!istate->cache_nr)
+               return 0;
+
+       cmp_last = strcmp_offset(name,
+                                istate->cache[istate->cache_nr - 1]->name,
+                                &len_eq_last);
+       if (cmp_last > 0 && name[len_eq_last] != '/')
+               /*
+                * The entry sorts AFTER the last one in the
+                * index and their paths have no common prefix,
+                * so there cannot be a F/D conflict.
+                */
+               return 0;
 
        for (;;) {
                size_t len;