git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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 1/3] add -p: avoid ambiguous signed/unsigned comparison
Date: Tue, 30 Aug 2022 13:54:22 +0000	[thread overview]
Message-ID: <0691d7eaaa03e8bf8b460b9e20ec05eec09fb574.1661867664.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1340.git.1661867664.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

In the interactive `add` operation, users can choose to jump to specific
hunks, and Git will present the hunk list in that case. To avoid showing
too many lines at once, only a maximum of 21 hunks are shown, skipping
the "mode change" pseudo hunk.

The comparison performed to skip the "mode change" pseudo hunk (if any)
compares a signed integer `i` to the unsigned value `mode_change` (which
can be 0 or 1 because it is a 1-bit type).

According to section 6.3.1.8 of the C99 standard (see e.g.
https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf), what should
happen is an automatic conversion of the "lesser" type to the "greater"
type, but since the types differ in signedness, it is ill-defined what
is the correct "usual arithmetic conversion".

Which means that Visual C's behavior can (and does) differ from GCC's:
When compiling Git using the latter, `add -p`'s `goto` command shows no
hunks by default because it casts a negative start offset to a pretty
large unsigned value, breaking the "goto hunk" test case in
`t3701-add-interactive.sh`.

Let's avoid that by converting the unsigned bit explicitly to a signed
integer.

Note: This is a long-standing bug in the Visual C build of Git, but it
has never been caught because t3701 is skipped when `NO_PERL` is set,
which is the case in the `vs-test` jobs of Git's CI runs.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 add-patch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/add-patch.c b/add-patch.c
index 509ca04456b..3524555e2b0 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1547,7 +1547,7 @@ soft_increment:
 			strbuf_remove(&s->answer, 0, 1);
 			strbuf_trim(&s->answer);
 			i = hunk_index - DISPLAY_HUNKS_LINES / 2;
-			if (i < file_diff->mode_change)
+			if (i < (int)file_diff->mode_change)
 				i = file_diff->mode_change;
 			while (s->answer.len == 0) {
 				i = display_hunks(s, file_diff, i);
-- 
gitgitgadget


  reply	other threads:[~2022-08-30 13:55 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-30 13:54 [PATCH 0/3] A couple of CI fixes regarding the built-in add --patch Johannes Schindelin via GitGitGadget
2022-08-30 13:54 ` Johannes Schindelin via GitGitGadget [this message]
2022-08-30 19:09   ` [PATCH 1/3] add -p: avoid ambiguous signed/unsigned comparison Junio C Hamano
2022-08-30 13:54 ` [PATCH 2/3] t3701: test the built-in `add -i` regardless of NO_PERL Johannes Schindelin via GitGitGadget
2022-08-30 18:54   ` Jeff King
2022-08-30 21:03     ` Johannes Schindelin
2022-08-30 19:09   ` Junio C Hamano
2022-08-30 13:54 ` [PATCH 3/3] t6132(NO_PERL): do not run the scripted `add -p` Johannes Schindelin via GitGitGadget
2022-08-30 14:19 ` validating signed/unsigned comparisons with Coccinelle, was Re: [PATCH 0/3] A couple of CI fixes regarding the built-in add --patch Johannes Schindelin
2022-08-30 15:22   ` Phillip Wood
2022-08-30 21:12     ` Johannes Schindelin
2022-08-30 21:29       ` Junio C Hamano
2022-08-30 21:46         ` Junio C Hamano
2022-08-30 21:32       ` Jeff King
2022-08-30 15:19 ` Phillip Wood

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=0691d7eaaa03e8bf8b460b9e20ec05eec09fb574.1661867664.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.
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).