Re: [PATCH 7/7] odb: use size_t for object_info.sizep and the size APIs

[Patrick Steinhardt <ps@pks.im>]

On Thu, Jun 04, 2026 at 10:51:12AM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index fa45f774d7..fa6e396ddc 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -120,7 +120,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  	struct object_id oid;
>  	enum object_type type;
>  	char *buf;
> -	unsigned long size;
> +	size_t size;
>  	struct object_context obj_context = {0};
>  	struct object_info oi = OBJECT_INFO_INIT;
>  	unsigned flags = OBJECT_INFO_LOOKUP_REPLACE;
> @@ -166,7 +166,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  		if (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {
>  			size_t s = size;
>  			buf = replace_idents_using_mailmap(buf, &s);
> -			size = cast_size_t_to_ulong(s);
> +			size = s;
>  		}
>  
>  		printf("%"PRIuMAX"\n", (uintmax_t)size);

Can't we drop this local variable completely and instead supply `&size`
directly?

> @@ -219,7 +225,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  		if (use_mailmap) {
>  			size_t s = size;
>  			buf = replace_idents_using_mailmap(buf, &s);
> -			size = cast_size_t_to_ulong(s);
> +			size = s;
>  		}
>  
>  		/* otherwise just spit out the data */
> @@ -266,7 +272,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
>  		if (use_mailmap) {
>  			size_t s = size;
>  			buf = replace_idents_using_mailmap(buf, &s);
> -			size = cast_size_t_to_ulong(s);
> +			size = s;
>  		}
>  		break;
>  	}
> @@ -446,7 +455,7 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
>  		if (use_mailmap) {
>  			size_t s = size;
>  			contents = replace_idents_using_mailmap(contents, &s);
> -			size = cast_size_t_to_ulong(s);
> +			size = s;
>  		}
>  
>  		if (type != data->type)

Likewise for these three instances.

> @@ -555,7 +564,7 @@ static void batch_object_write(const char *obj_name,
>  			if (!buf)
>  				die(_("unable to read %s"), oid_to_hex(&data->oid));
>  			buf = replace_idents_using_mailmap(buf, &s);
> -			data->size = cast_size_t_to_ulong(s);
> +			data->size = s;
>  
>  			free(buf);
>  		}

And I think this site here can be adapted, as well.

> diff --git a/diff.c b/diff.c
> index 5a584fa1d5..816b89dc6c 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4594,8 +4594,9 @@ int diff_populate_filespec(struct repository *r,
>  		}
>  	}
>  	else {
> +		size_t size_st = 0;
>  		struct object_info info = {
> -			.sizep = &s->size
> +			.sizep = &size_st
>  		};
>  
>  		if (!(size_only || check_binary))
> @@ -4617,6 +4618,7 @@ int diff_populate_filespec(struct repository *r,
>  			die("unable to read %s", oid_to_hex(&s->oid));
>  
>  object_read:
> +		s->size = cast_size_t_to_ulong(size_st);
>  		if (size_only || check_binary) {
>  			if (size_only)
>  				return 0;
> @@ -4631,6 +4633,7 @@ object_read:
>  			if (odb_read_object_info_extended(r->objects, &s->oid, &info,
>  							  OBJECT_INFO_LOOKUP_REPLACE))
>  				die("unable to read %s", oid_to_hex(&s->oid));
> +			s->size = cast_size_t_to_ulong(size_st);
>  		}
>  		s->should_free = 1;
>  	}

The flow in this function is quite weird if you ask me, but that's a
preexisting issue. This does look correct to me, even if it's awkward.

Patrick