[PATCH v2] wt-status: correct and simplify check for detached HEAD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



If a branch name is longer than four characters then memcmp() reads over
the end of the static string "HEAD".  This causes the following test
failures with AddressSanitizer:

t3203-branch-output.sh                           (Wstat: 256 Tests: 18 Failed: 4)
  Failed tests:  12, 15-17
  Non-zero exit status: 1
t3412-rebase-root.sh                             (Wstat: 256 Tests: 31 Failed: 3)
  Failed tests:  28-29, 31
  Non-zero exit status: 1
t3507-cherry-pick-conflict.sh                    (Wstat: 256 Tests: 31 Failed: 4)
  Failed tests:  14, 29-31
  Non-zero exit status: 1
t3510-cherry-pick-sequence.sh                    (Wstat: 256 Tests: 39 Failed: 14)
  Failed tests:  17, 22-26, 28-30, 34-35, 37-39
  Non-zero exit status: 1
t3420-rebase-autostash.sh                        (Wstat: 256 Tests: 28 Failed: 4)
  Failed tests:  24-27
  Non-zero exit status: 1
t3404-rebase-interactive.sh                      (Wstat: 256 Tests: 91 Failed: 57)
  Failed tests:  17, 19, 21-42, 44, 46-74, 77, 81-82
  Non-zero exit status: 1
t3900-i18n-commit.sh                             (Wstat: 256 Tests: 34 Failed: 1)
  Failed test:  34
  Non-zero exit status: 1
t5407-post-rewrite-hook.sh                       (Wstat: 256 Tests: 14 Failed: 6)
  Failed tests:  9-14
  Non-zero exit status: 1
t7001-mv.sh                                      (Wstat: 256 Tests: 46 Failed: 5)
  Failed tests:  39-43
  Non-zero exit status: 1
t7509-commit.sh                                  (Wstat: 256 Tests: 12 Failed: 2)
  Failed tests:  11-12
  Non-zero exit status: 1
t7512-status-help.sh                             (Wstat: 256 Tests: 39 Failed: 35)
  Failed tests:  5-39
  Non-zero exit status: 1
t6030-bisect-porcelain.sh                        (Wstat: 256 Tests: 70 Failed: 1)
  Failed test:  13
  Non-zero exit status: 1

And if a branch is named "H", "HE", or "HEA" then the current if clause
erroneously considers it as matching "HEAD" because it only compares
up to the end of the branch name.

Fix that by doing the comparison using strcmp() and only after the
branch name is extracted.  This way neither too less nor too many
characters are checked.  While at it call strchrnul() to find the end
of the branch name instead of open-coding it.

Helped-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Rene Scharfe <l.s.r@xxxxxx>
---
We can be more careful when parsing -- or avoid parsing and backtrack
if we found "HEAD" after all.  The latter is simpler, and string
parsing is tricky enough that we better take such opportunities to
simplify the code..

Changes since v1:
* strcmp() instead of strncmp()
* strchrnul() (unrelated cleanup)
* adjusted subject (and commit message)

 wt-status.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 435fc28..ced53dd 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1317,15 +1317,14 @@ static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
 	target += strlen(" to ");
 	strbuf_reset(&cb->buf);
 	hashcpy(cb->nsha1, nsha1);
-	for (end = target; *end && *end != '\n'; end++)
-		;
-	if (!memcmp(target, "HEAD", end - target)) {
+	end = strchrnul(target, '\n');
+	strbuf_add(&cb->buf, target, end - target);
+	if (!strcmp(cb->buf.buf, "HEAD")) {
 		/* HEAD is relative. Resolve it to the right reflog entry. */
+		strbuf_reset(&cb->buf);
 		strbuf_addstr(&cb->buf,
 			      find_unique_abbrev(nsha1, DEFAULT_ABBREV));
-		return 1;
 	}
-	strbuf_add(&cb->buf, target, end - target);
 	return 1;
 }
 
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]