Tuomas Ahola <taahol@xxxxxx> writes:
> In file bulk-checkin.c, three warnings are emitted by
> "-Wsign-compare", two of which are caused by trivial loop iterator
> type mismatches. The third one is also an uncomplicated case for
> which a simple cast is a sufficient remedy.
>
Nit: it would be nice if you expanded on why 'a simple cast is a
sufficient remedy' and more importantly how that casting is safe.
> Fix issues accordingly, and enable sign compare warnings for the file.
>
> Signed-off-by: Tuomas Ahola <taahol@xxxxxx>
> ---
> bulk-checkin.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/bulk-checkin.c b/bulk-checkin.c
> index 20f2da67b9..0133427132 100644
> --- a/bulk-checkin.c
> +++ b/bulk-checkin.c
> @@ -3,7 +3,6 @@
> */
>
> #define USE_THE_REPOSITORY_VARIABLE
> -#define DISABLE_SIGN_COMPARE_WARNINGS
>
> #include "git-compat-util.h"
> #include "bulk-checkin.h"
> @@ -56,7 +55,6 @@ static void flush_bulk_checkin_packfile(struct bulk_checkin_packfile *state)
> {
> unsigned char hash[GIT_MAX_RAWSZ];
> struct strbuf packname = STRBUF_INIT;
> - int i;
>
> if (!state->f)
> return;
> @@ -82,7 +80,7 @@ static void flush_bulk_checkin_packfile(struct bulk_checkin_packfile *state)
> finish_tmp_packfile(&packname, state->pack_tmp_name,
> state->written, state->nr_written,
> &state->pack_idx_opts, hash);
> - for (i = 0; i < state->nr_written; i++)
> + for (uint32_t i = 0; i < state->nr_written; i++)
> free(state->written[i]);
>
> clear_exit:
> @@ -131,14 +129,12 @@ static void flush_batch_fsync(void)
>
> static int already_written(struct bulk_checkin_packfile *state, struct object_id *oid)
> {
> - int i;
> -
> /* The object may already exist in the repository */
> if (repo_has_object_file(the_repository, oid))
> return 1;
>
> /* Might want to keep the list sorted */
> - for (i = 0; i < state->nr_written; i++)
> + for (uint32_t i = 0; i < state->nr_written; i++)
> if (oideq(&state->written[i]->oid, oid))
> return 1;
>
> @@ -192,7 +188,7 @@ static int stream_blob_to_pack(struct bulk_checkin_packfile *state,
> offset += rsize;
> if (*already_hashed_to < offset) {
> size_t hsize = offset - *already_hashed_to;
> - if (rsize < hsize)
> + if ((size_t)rsize < hsize)
Something I found peculiar here is that `rsize` is of type ssize_t'.
But it only seems to store a positive value.
> hsize = rsize;
> if (hsize)
> git_hash_update(ctx, ibuf, hsize);
>
> base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
> --
> 2.30.2
Attachment:
signature.asc
Description: PGP signature
