| Message ID | 20250208182736.18133-2-dhar61595@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix -Wsign-compare warnings in refs.c | expand |
On Sat, Feb 8, 2025, at 19:27, Moumita wrote: > Remove DISABLE_SIGN_COMPARE_WARNINGS from refs.c and fix integer > comparison issues that caused -Wsign-compare warnings. > > Tested using `make DEVELOPER=1` and `make t` to ensure correctness. Notes on how the patch/commit was tested is usually not included in the commit message. > --- You forgot the signoff.
Moumita <dhar61595@gmail.com> writes: > Remove DISABLE_SIGN_COMPARE_WARNINGS from refs.c and fix integer > comparison issues that caused -Wsign-compare warnings. > > Tested using `make DEVELOPER=1` and `make t` to ensure correctness. > --- > refs.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) I think this is wrong. The string-list structure is screwed up in its use of types. The alloc/nr pair to keep track of the number and insert index of the elements were updated to size_t in 2022 but that change was incomplete. The API function string_list_find_insert_index() that returns the index into list still return "int", and that is what needs to be corrected, no? > diff --git a/refs.c b/refs.c > index 37b8cfb90c..e5cb7acfbe 100644 > --- a/refs.c > +++ b/refs.c > @@ -3,7 +3,7 @@ > */ > > #define USE_THE_REPOSITORY_VARIABLE > -#define DISABLE_SIGN_COMPARE_WARNINGS > + > > #include "git-compat-util.h" > #include "advice.h" > @@ -1652,7 +1652,7 @@ const char *find_descendant_ref(const char *dirname, > * slash) and is not in skip, then we have a conflict. > */ > for (pos = string_list_find_insert_index(extras, dirname, 0); > - pos < extras->nr; pos++) { > + (size_t)pos < extras->nr; pos++) { > const char *extra_refname = extras->items[pos].string; > @@ -2304,7 +2304,8 @@ static int run_transaction_hook(struct ref_transaction *transaction, > struct child_process proc = CHILD_PROCESS_INIT; > struct strbuf buf = STRBUF_INIT; > const char *hook; > - int ret = 0, i; > + int ret = 0; > + size_t i ; This may be OK, as .nr is of size_t. > hook = find_hook(transaction->ref_store->repo, "reference-transaction"); > if (!hook) > @@ -2635,9 +2636,9 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, > ref_transaction_for_each_queued_update_fn cb, > void *cb_data) > { > - int i; > + size_t i; > > - for (i = 0; i < transaction->nr; i++) { > + for ( i = 0; i < transaction->nr; i++) { It is also wrong to add SP there. > struct ref_update *update = transaction->updates[i]; > > cb(update->refname,
diff --git a/refs.c b/refs.c index 37b8cfb90c..e5cb7acfbe 100644 --- a/refs.c +++ b/refs.c @@ -3,7 +3,7 @@ */ #define USE_THE_REPOSITORY_VARIABLE -#define DISABLE_SIGN_COMPARE_WARNINGS + #include "git-compat-util.h" #include "advice.h" @@ -1652,7 +1652,7 @@ const char *find_descendant_ref(const char *dirname, * slash) and is not in skip, then we have a conflict. */ for (pos = string_list_find_insert_index(extras, dirname, 0); - pos < extras->nr; pos++) { + (size_t)pos < extras->nr; pos++) { const char *extra_refname = extras->items[pos].string; if (!starts_with(extra_refname, dirname)) @@ -2304,7 +2304,8 @@ static int run_transaction_hook(struct ref_transaction *transaction, struct child_process proc = CHILD_PROCESS_INIT; struct strbuf buf = STRBUF_INIT; const char *hook; - int ret = 0, i; + int ret = 0; + size_t i ; hook = find_hook(transaction->ref_store->repo, "reference-transaction"); if (!hook) @@ -2635,9 +2636,9 @@ void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, ref_transaction_for_each_queued_update_fn cb, void *cb_data) { - int i; + size_t i; - for (i = 0; i < transaction->nr; i++) { + for ( i = 0; i < transaction->nr; i++) { struct ref_update *update = transaction->updates[i]; cb(update->refname,