git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/3] add -p: avoid ambiguous signed/unsigned comparison

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 30, 2022, 19:09 UTC
Message-ID
<xmqqtu5tk0pi.fsf@gitster.g>
In-Reply-To
<0691d7eaaa03e8bf8b460b9e20ec05eec09fb574.1661867664.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 6 quoted lines
> 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.
Wow.

While the whole "add -i" was what I started, I admit that everything more recent than the support of the "edit" action in "add -p" is foreign to me. I didn't remember the existence of, and the justification for, the 20-line limit. It seems that this came from 3f6aff68 (Add subroutine to display one-line summary of hunks, 2008-12-04).

> 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).
OK, that warrants a casting to signed, surely.
> 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.
Good finding.
Show 18 quoted lines
> 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);
Previous: Johannes Schindelin via GitGitGadgetNext: Johannes Schindelin via GitGitGadget
Message 3 of 15 in “A couple of CI fixes regarding the built-in add --patch”
  1. 0/3 A couple of CI fixes regarding the built-in add --patchJohannes Schindelin via GitGitGadget, Aug 30, 2022
  2. 1/3 add -p: avoid ambiguous signed/unsigned comparisonJohannes Schindelin via GitGitGadget, Aug 30, 2022
  3. Junio C HamanoAug 30, 2022
  4. 2/3 t3701: test the built-in `add -i` regardless of NO_PERLJohannes Schindelin via GitGitGadget, Aug 30, 2022
  5. Jeff KingAug 30, 2022
  6. Johannes SchindelinAug 30, 2022
  7. Junio C HamanoAug 30, 2022
  8. 3/3 t6132(NO_PERL): do not run the scripted `add -p`Johannes Schindelin via GitGitGadget, Aug 30, 2022
  9. validating signed/unsigned comparisons with Coccinelle, was Re: [PATCH 0/3] A couple of CI fixes regarding the built-in add --patchJohannes Schindelin, Aug 30, 2022
  10. Phillip WoodAug 30, 2022
  11. Johannes SchindelinAug 30, 2022
  12. Junio C HamanoAug 30, 2022
  13. Junio C HamanoAug 30, 2022
  14. Jeff KingAug 30, 2022
  15. Phillip WoodAug 30, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.