Skip to content

Commit 6552684

Browse files
dschogitster
authored andcommitted
has_dir_name(): make code more obvious
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent bf0468e commit 6552684

File tree

1 file changed

+13
-42
lines changed

1 file changed

+13
-42
lines changed

read-cache.c

Lines changed: 13 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,48 +1117,19 @@ static int has_dir_name(struct index_state *istate,
11171117
*
11181118
* Compare the entry's full path with the last path in the index.
11191119
*/
1120-
if (istate->cache_nr > 0) {
1121-
cmp_last = strcmp_offset(name,
1122-
istate->cache[istate->cache_nr - 1]->name,
1123-
&len_eq_last);
1124-
if (cmp_last > 0) {
1125-
if (name[len_eq_last] != '/') {
1126-
/*
1127-
* The entry sorts AFTER the last one in the
1128-
* index.
1129-
*
1130-
* If there were a conflict with "file", then our
1131-
* name would start with "file/" and the last index
1132-
* entry would start with "file" but not "file/".
1133-
*
1134-
* The next character after common prefix is
1135-
* not '/', so there can be no conflict.
1136-
*/
1137-
return retval;
1138-
} else {
1139-
/*
1140-
* The entry sorts AFTER the last one in the
1141-
* index, and the next character after common
1142-
* prefix is '/'.
1143-
*
1144-
* Either the last index entry is a file in
1145-
* conflict with this entry, or it has a name
1146-
* which sorts between this entry and the
1147-
* potential conflicting file.
1148-
*
1149-
* In both cases, we fall through to the loop
1150-
* below and let the regular search code handle it.
1151-
*/
1152-
}
1153-
} else if (cmp_last == 0) {
1154-
/*
1155-
* The entry exactly matches the last one in the
1156-
* index, but because of multiple stage and CE_REMOVE
1157-
* items, we fall through and let the regular search
1158-
* code handle it.
1159-
*/
1160-
}
1161-
}
1120+
if (!istate->cache_nr)
1121+
return 0;
1122+
1123+
cmp_last = strcmp_offset(name,
1124+
istate->cache[istate->cache_nr - 1]->name,
1125+
&len_eq_last);
1126+
if (cmp_last > 0 && name[len_eq_last] != '/')
1127+
/*
1128+
* The entry sorts AFTER the last one in the
1129+
* index and their paths have no common prefix,
1130+
* so there cannot be a F/D conflict.
1131+
*/
1132+
return 0;
11621133

11631134
for (;;) {
11641135
size_t len;

0 commit comments

Comments
 (0)