git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	 Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 01/11] commit: simplify code
Date: Thu, 15 May 2025 13:37:00 -0700	[thread overview]
Message-ID: <xmqq4ixlzk43.fsf@gitster.g> (raw)
In-Reply-To: <20250515194806.GC3320240@coredump.intra.peff.net> (Jeff King's message of "Thu, 15 May 2025 15:48:06 -0400")

Jeff King <peff@peff.net> writes:

> ... there are any non-ita entries). Though in that case I'd think:
>
>   committable = 0;
>   for (i = 0; i < cache_nr; i++) {
> 	if (!ce_intent_to_add(...) {
> 		committable = 1;
> 		break;
> 	}
>   }
>
> would be the most clear, since we do not otherwise care about the actual
> number of ita entries. And lets us break out of the loop early.

Exactly.  If you focus on the warning too narrowly, the minimal
change in the original patch does look OK, but in the original (even
before Dscho's patch, that is) the intent is unclear, as opposed to
what you showed above.  And the update to squelch false positive
does not improve the clarity of the logic as the above rewrite does.

Thanks.

  reply	other threads:[~2025-05-15 20:37 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 [this message]
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 ` [PATCH 05/11] has_dir_name(): make code more obvious Johannes Schindelin via GitGitGadget
2025-05-15 20:04   ` 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=xmqq4ixlzk43.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    /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).