{"thread":{"id":"62925","subject":"[PATCH 0/1] [GSoC] [Newbie] Fix -Wsign-compare warnings in refs.c","startedAt":"2025-02-08T18:29:17Z","lastAt":"2025-02-08T22:07:49Z","messageCount":4,"participants":["Moumita","Kristoffer Haugsbakk","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"512136","messageId":"20250208182736.18133-1-dhar61595@gmail.com","threadId":"62925","inReplyTo":null,"subject":"[PATCH 0/1] [GSoC] [Newbie] Fix -Wsign-compare warnings in refs.c","fromName":"Moumita","fromEmail":"dhar61595@gmail.com","sentAt":"2025-02-08T18:27:34Z","receivedAt":"2025-02-08T18:29:17Z","isPatch":true,"sender":{"key":"dhar61595@gmail.com","avatar":"https://avatars.githubusercontent.com/u/101038146?v=4"},"body":"Hello Git community,\n\nI am a newcomer interested in contributing to Git as part of GSoC. This patch\nremoves `DISABLE_SIGN_COMPARE_WARNINGS` from `refs.c` and fixes integer\ncomparison issues that caused `-Wsign-compare` warnings.\n\nMoumita (1):\n refs.c: fix -Wsign-compare warnings\n\n refs.c | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\n-- \n2.48.0\n\n"},{"id":"512137","messageId":"20250208182736.18133-2-dhar61595@gmail.com","threadId":"62925","inReplyTo":"20250208182736.18133-1-dhar61595@gmail.com","subject":"[PATCH 1/1] refs.c: fix -Wsign-compare warnings","fromName":"Moumita","fromEmail":"dhar61595@gmail.com","sentAt":"2025-02-08T18:27:35Z","receivedAt":"2025-02-08T18:29:31Z","isPatch":true,"sender":{"key":"dhar61595@gmail.com","avatar":"https://avatars.githubusercontent.com/u/101038146?v=4"},"body":"Remove DISABLE_SIGN_COMPARE_WARNINGS from refs.c and fix integer\ncomparison issues that caused -Wsign-compare warnings.\n\nTested using `make DEVELOPER=1` and `make t` to ensure correctness.\n---\n refs.c | 11 ++++++-----\n 1 file changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 37b8cfb90c..e5cb7acfbe 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3,7 +3,7 @@\n */\n \n #define USE_THE_REPOSITORY_VARIABLE\n-#define DISABLE_SIGN_COMPARE_WARNINGS\n+\n \n #include \"git-compat-util.h\"\n #include \"advice.h\"\n@@ -1652,7 +1652,7 @@ const char *find_descendant_ref(const char *dirname,\n \t * slash) and is not in skip, then we have a conflict.\n \t */\n \tfor (pos = string_list_find_insert_index(extras, dirname, 0);\n-\t pos < extras->nr; pos++) {\n+\t (size_t)pos < extras->nr; pos++) {\n \t\tconst char *extra_refname = extras->items[pos].string;\n \n \t\tif (!starts_with(extra_refname, dirname))\n@@ -2304,7 +2304,8 @@ static int run_transaction_hook(struct ref_transaction *transaction,\n \tstruct child_process proc = CHILD_PROCESS_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n \tconst char *hook;\n-\tint ret = 0, i;\n+\tint ret = 0;\n+\tsize_t i ;\n \n \thook = find_hook(transaction->ref_store->repo, \"reference-transaction\");\n \tif (!hook)\n@@ -2635,9 +2636,9 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction,\n \t\t\t\t\t ref_transaction_for_each_queued_update_fn cb,\n \t\t\t\t\t void *cb_data)\n {\n-\tint i;\n+\tsize_t i;\n \n-\tfor (i = 0; i < transaction->nr; i++) {\n+\tfor ( i = 0; i < transaction->nr; i++) {\n \t\tstruct ref_update *update = transaction->updates[i];\n \n \t\tcb(update->refname,\n-- \n2.48.0\n\n"},{"id":"512138","messageId":"190f7624-d45e-4c5f-a29a-b3be84e113d1@app.fastmail.com","threadId":"62925","inReplyTo":"20250208182736.18133-2-dhar61595@gmail.com","subject":"Re: [PATCH 1/1] refs.c: fix -Wsign-compare warnings","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-02-08T19:01:51Z","receivedAt":"2025-02-08T19:02:31Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Sat, Feb 8, 2025, at 19:27, Moumita wrote:\n> Remove DISABLE_SIGN_COMPARE_WARNINGS from refs.c and fix integer\n> comparison issues that caused -Wsign-compare warnings.\n>\n> Tested using `make DEVELOPER=1` and `make t` to ensure correctness.\n\nNotes on how the patch/commit was tested is usually not \nincluded in the commit message.\n\n> ---\n\nYou forgot the signoff.\n"},{"id":"512145","messageId":"xmqqa5awdr9p.fsf@gitster.g","threadId":"62925","inReplyTo":"20250208182736.18133-2-dhar61595@gmail.com","subject":"Re: [PATCH 1/1] refs.c: fix -Wsign-compare warnings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-08T22:07:46Z","receivedAt":"2025-02-08T22:07:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Moumita writes:\n\n> Remove DISABLE_SIGN_COMPARE_WARNINGS from refs.c and fix integer\n> comparison issues that caused -Wsign-compare warnings.\n>\n> Tested using `make DEVELOPER=1` and `make t` to ensure correctness.\n> ---\n> refs.c | 11 ++++++-----\n> 1 file changed, 6 insertions(+), 5 deletions(-)\n\nI think this is wrong.\n\nThe string-list structure is screwed up in its use of types.\n\nThe alloc/nr pair to keep track of the number and insert index of\nthe elements were updated to size_t in 2022 but that change was\nincomplete. The API function string_list_find_insert_index() that\nreturns the index into list still return \"int\", and that is what\nneeds to be corrected, no?\n\n> diff --git a/refs.c b/refs.c\n> index 37b8cfb90c..e5cb7acfbe 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -3,7 +3,7 @@\n> */\n> \n> #define USE_THE_REPOSITORY_VARIABLE\n> -#define DISABLE_SIGN_COMPARE_WARNINGS\n> +\n> \n> #include \"git-compat-util.h\"\n> #include \"advice.h\"\n> @@ -1652,7 +1652,7 @@ const char *find_descendant_ref(const char *dirname,\n> \t * slash) and is not in skip, then we have a conflict.\n> \t */\n> \tfor (pos = string_list_find_insert_index(extras, dirname, 0);\n> -\t pos < extras->nr; pos++) {\n> +\t (size_t)pos < extras->nr; pos++) {\n> \t\tconst char *extra_refname = extras->items[pos].string;\n\n\n\n\n> @@ -2304,7 +2304,8 @@ static int run_transaction_hook(struct ref_transaction *transaction,\n> \tstruct child_process proc = CHILD_PROCESS_INIT;\n> \tstruct strbuf buf = STRBUF_INIT;\n> \tconst char *hook;\n> -\tint ret = 0, i;\n> +\tint ret = 0;\n> +\tsize_t i ;\n\nThis may be OK, as .nr is of size_t.\n\n> \thook = find_hook(transaction->ref_store->repo, \"reference-transaction\");\n> \tif (!hook)\n> @@ -2635,9 +2636,9 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction,\n> \t\t\t\t\t ref_transaction_for_each_queued_update_fn cb,\n> \t\t\t\t\t void *cb_data)\n> {\n> -\tint i;\n> +\tsize_t i;\n> \n> -\tfor (i = 0; i < transaction->nr; i++) {\n> +\tfor ( i = 0; i < transaction->nr; i++) {\n\nIt is also wrong to add SP there.\n\n> \t\tstruct ref_update *update = transaction->updates[i];\n> \n> \t\tcb(update->refname,\n"}]}