From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH 05/11] has_dir_name(): make code more obvious
Date: Thu, 15 May 2025 13:11:43 +0000 [thread overview]
Message-ID: <80422a5770ded04993c73c657b363ddad45e2f4a.1747314709.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1891.git.1747314709.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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>
---
read-cache.c | 55 +++++++++++++---------------------------------------
1 file changed, 13 insertions(+), 42 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 73f83a7e7a11..c0bb760ad473 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -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;
--
gitgitgadget
next prev parent reply other threads:[~2025-05-15 13:11 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-15 13:11 [PATCH 00/11] CodeQL-inspired fixes Johannes Schindelin via GitGitGadget
2025-05-15 13:11 ` [PATCH 01/11] commit: simplify code Johannes Schindelin via GitGitGadget
2025-05-15 19:48 ` Jeff King
2025-05-15 20:37 ` Junio C Hamano
2025-05-15 20:49 ` Jeff King
2025-05-15 13:11 ` [PATCH 02/11] fetch: carefully clear local variable's address after use Johannes Schindelin via GitGitGadget
2025-05-15 19:40 ` Jeff King
2025-05-15 13:11 ` [PATCH 03/11] commit-graph: avoid malloc'ing a local variable Johannes Schindelin via GitGitGadget
2025-05-15 19:54 ` Jeff King
2025-05-15 21:40 ` Junio C Hamano
2025-05-15 13:11 ` [PATCH 04/11] upload-pack: rename `enum` to reflect the operation Johannes Schindelin via GitGitGadget
2025-05-15 19:55 ` Jeff King
2025-05-15 13:11 ` Johannes Schindelin via GitGitGadget [this message]
2025-05-15 20:04 ` [PATCH 05/11] has_dir_name(): make code more obvious Jeff King
2025-05-15 13:11 ` [PATCH 06/11] fetch: avoid unnecessary work when there is no current branch Johannes Schindelin via GitGitGadget
2025-05-15 20:11 ` Jeff King
2025-05-15 13:11 ` [PATCH 07/11] Avoid redundant conditions Johannes Schindelin via GitGitGadget
2025-05-15 20:13 ` Jeff King
2025-05-15 13:11 ` [PATCH 08/11] trace2: avoid "futile conditional" Johannes Schindelin via GitGitGadget
2025-05-15 20:16 ` Jeff King
2025-05-15 13:11 ` [PATCH 09/11] commit-graph: avoid using stale stack addresses Johannes Schindelin via GitGitGadget
2025-05-15 20:19 ` Jeff King
2025-05-15 13:11 ` [PATCH 10/11] bundle-uri: avoid using undefined output of `sscanf()` Johannes Schindelin via GitGitGadget
2025-05-15 19:21 ` Junio C Hamano
2025-05-15 20:25 ` Jeff King
2025-05-16 10:11 ` Phillip Wood
2025-05-16 13:40 ` Phillip Wood
2025-05-16 15:42 ` Jeff King
2025-05-19 9:03 ` Phillip Wood
2025-05-22 6:03 ` Jeff King
2025-05-15 13:11 ` [PATCH 11/11] sequencer: stop pretending that an assignment is a condition Johannes Schindelin via GitGitGadget
2025-05-15 18:51 ` Junio C Hamano
2025-05-15 20:26 ` Jeff King
2025-05-16 10:13 ` Phillip Wood
2025-05-15 20:26 ` [PATCH 00/11] CodeQL-inspired fixes Jeff King
2025-05-15 20:58 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=80422a5770ded04993c73c657b363ddad45e2f4a.1747314709.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=johannes.schindelin@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).