From: Eric Sunshine <sunshine@sunshineco.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
Hariom verma <hariom18599@gmail.com>,
Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH] worktree: avoid dead-code in conditional
Date: Wed, 24 Jun 2020 15:05:41 -0400 [thread overview]
Message-ID: <20200624190541.5253-1-sunshine@sunshineco.com> (raw)
get_worktrees() retrieves a list of all worktrees associated with a
repository, including the main worktree. The location of the main
worktree is determined by get_main_worktree() which needs to handle
three distinct cases for the main worktree after absolute-path
conversion:
* <bare-repository>/.
* <main-worktree>/.git/. (when $CWD is .git)
* <main-worktree>/.git (when $CWD is any worktree)
They all need to be normalized to just the <path> portion, dropping any
"/." or "/.git" suffix.
It turns out, however, that get_main_worktree() was only handling the
first and last cases, i.e.:
if (!strip_suffix(path, "/.git"))
strip_suffix(path, "/.");
This shortcoming was addressed by 45f274fbb1 (get_main_worktree(): allow
it to be called in the Git directory, 2020-02-23) by changing the logic
to:
strip_suffix(path, "/.");
if (!strip_suffix(path, "/.git"))
strip_suffix(path, "/.");
which makes the final strip_suffix() invocation dead-code.
Fix this oversight by enumerating the three distinct cases explicitly
rather than attempting to strip the suffix(es) incrementally.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
This "dead-code problem" was caught during review[1], however, the patch
was never re-rolled and ended up being accepted as-is. Dscho had raised
an objection[2] to the suggested way to fix the dead-code problem but
his objection was based upon a misunderstanding[3] thus (implicitly)
withdrawn.
An alternative to enumerating the three distinct cases, as this patch
does, would be to strip suffix components incrementally like this:
strip_suffix(path, "/.");
strip_suffix(path, "/.git");
However, such code is relatively opaque and would require a largish
in-code comment mirroring a chunk of the comment message of this patch.
The chosen approach, on the other hand, is relatively self-documenting,
requiring only very small inline source-code comments.
[1]: https://lore.kernel.org/git/CAPig+cTh-uu-obh9aeDOV9ptbVwRmkujgucbu9ei1Qa3qSNG_A@mail.gmail.com/
[2]: https://lore.kernel.org/git/nycvar.QRO.7.76.6.2002241942120.46@tvgsbejvaqbjf.bet/
[3]: https://lore.kernel.org/git/nycvar.QRO.7.76.6.2002271658250.46@tvgsbejvaqbjf.bet/
worktree.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/worktree.c b/worktree.c
index ee82235f26..b5cdee34de 100644
--- a/worktree.c
+++ b/worktree.c
@@ -50,9 +50,9 @@ static struct worktree *get_main_worktree(void)
struct strbuf worktree_path = STRBUF_INIT;
strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
- strbuf_strip_suffix(&worktree_path, "/.");
- if (!strbuf_strip_suffix(&worktree_path, "/.git"))
- strbuf_strip_suffix(&worktree_path, "/.");
+ if (!strbuf_strip_suffix(&worktree_path, "/.git/.") && /* in .git */
+ !strbuf_strip_suffix(&worktree_path, "/.git")) /* in worktree */
+ strbuf_strip_suffix(&worktree_path, "/."); /* in bare repo */
worktree = xcalloc(1, sizeof(*worktree));
worktree->path = strbuf_detach(&worktree_path, NULL);
--
2.27.0.288.g4b34aa94c7
next reply other threads:[~2020-06-24 19:06 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-24 19:05 Eric Sunshine [this message]
2020-06-24 21:20 ` [PATCH] worktree: avoid dead-code in conditional Junio C Hamano
2020-06-24 23:00 ` Eric Sunshine
2020-06-25 0:38 ` 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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200624190541.5253-1-sunshine@sunshineco.com \
--to=sunshine@sunshineco.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=hariom18599@gmail.com \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).