diff mbox series

[v4] git: replace greater-than and less-than checks with one not equal check

Message ID pull.1432.v4.git.git.1734489859673.gitgitgadget@gmail.com (mailing list archive)
State New
Headers show
Series [v4] git: replace greater-than and less-than checks with one not equal check | expand

Commit Message

Seija Kijin Dec. 18, 2024, 2:44 a.m. UTC 
From: Seija Kijin <doremylover123@gmail.com>

(version < 2 || version > 2) looks silly
considering this is an integer.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    git: replace greater-than and less-than checks with one not equal check
    
    (version < 2 || version > 2) looks silly considering this is an integer.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1432%2FAreaZR%2Fversion-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1432/AreaZR/version-v4
Pull-Request: https://github.com/git/git/pull/1432

Range-diff vs v3:

 1:  322d04519d5 = 1:  503f6635522 git: replace greater-than and less-than checks with one not equal check


 builtin/show-index.c | 2 +-
 packfile.c           | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


base-commit: 063bcebf0c917140ca0e705cbe0fdea127e90086

Comments

Junio C Hamano Dec. 18, 2024, 3:39 p.m. UTC | #1 
"AreaZR via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  	if (top_index[0] == htonl(PACK_IDX_SIGNATURE)) {
>  		version = ntohl(top_index[1]);
> -		if (version < 2 || version > 2)
> +		if (version != 2)
>  			die("unknown index version");

I am of two minds.  If the code never evolves and we will never
support anything other than version #2, your rewrite certainly makes
it easier to read.  On the other hand, if we plan to ever learn to
grok versions #3 and later, the original would be easier to se what
is going on, i.e.

		if (version < VERSION_LB || VERSION_UB < version)
			die("version out of bounds");

and the code as written happens to have "2" as both lower- and
upper-bound.

Of course when we do introduce version #3, this line must be updated
anyway, but the final form would be as we have it with the second
"2" replaced with "3", so leaving it in the current shape may be
easier for the developer doing that work.

So I do not know if the proposed change is an improvement for the
longer term.
diff mbox series

Patch

diff --git a/builtin/show-index.c b/builtin/show-index.c
index f164c01bbea..5fb71a1c425 100644
--- a/builtin/show-index.c
+++ b/builtin/show-index.c
@@ -44,7 +44,7 @@  int cmd_show_index(int argc,
 		die("unable to read header");
 	if (top_index[0] == htonl(PACK_IDX_SIGNATURE)) {
 		version = ntohl(top_index[1]);
-		if (version < 2 || version > 2)
+		if (version != 2)
 			die("unknown index version");
 		if (fread(top_index, 256 * 4, 1, stdin) != 1)
 			die("unable to read index");
diff --git a/packfile.c b/packfile.c
index 9c4bd81a8c7..de0662b2353 100644
--- a/packfile.c
+++ b/packfile.c
@@ -114,7 +114,7 @@  int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
 
 	if (hdr->idx_signature == htonl(PACK_IDX_SIGNATURE)) {
 		version = ntohl(hdr->idx_version);
-		if (version < 2 || version > 2)
+		if (version != 2)
 			return error("index file %s is version %"PRIu32
 				     " and is not supported by this binary"
 				     " (try upgrading GIT to a newer version)",