Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions apply.c
Original file line number Diff line number Diff line change
Expand Up @@ -3232,7 +3232,7 @@ static int apply_binary_fragment(struct apply_state *state,
struct patch *patch)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):

On Thu, Jun 04, 2026 at 10:51:07AM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> `patch_delta()` takes the source and delta sizes by value and writes
> back the reconstructed target size through an `unsigned long *`.  That
> datatype cannot represent a value that exceeds 4 GiB on systems where
> `unsigned long` is 32-bit (notably 64-bit Windows builds), though, even
> though the delta encoding itself, the on-disk layout, and the in-memory
> buffers happily carry such sizes. A `size_t` companion to
> `get_delta_hdr_size()`, `get_delta_hdr_size_sz()`, was introduced in
> 17fa077596 (delta, packfile: use size_t for delta header sizes,
> 2026-05-08) precisely so that `patch_delta()` could be widened without
> changing the on-the-wire decoding helper's signature.
> 
> Widen `patch_delta()`'s three size parameters to `size_t` and switch
> its internal use of `get_delta_hdr_size()` to the `_sz` variant.
> Then propagate the wider type through the callers.

Does `get_delta_hdr_size()` have any remaining callers after this patch
series? I currently only spot two such callers, and you convert both of
them in this patch.

And can we reasonably add a test case that exercises this change?

> diff --git a/packfile.c b/packfile.c
> index 89366abfe3..e202f48837 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -1964,10 +1964,8 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
>  			      (uintmax_t)curpos, p->pack_name);
>  			data = NULL;
>  		} else {
> -			unsigned long sz;
>  			data = patch_delta(base, base_size, delta_data,
> -					   delta_size, &sz);
> -			size = sz;
> +					   delta_size, &size);

Nice that we get rid of this awkward construct.

Patrick

{
struct fragment *fragment = patch->fragments;
unsigned long len;
size_t len;
void *dst;

if (!fragment)
Expand Down Expand Up @@ -3321,7 +3321,7 @@ static int apply_binary(struct apply_state *state,
if (odb_has_object(the_repository->objects, &oid, 0)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):

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

/* We already have the postimage */
enum object_type type;
unsigned long size;
size_t size;
char *result;

result = odb_read_object(the_repository->objects, &oid,
Expand Down Expand Up @@ -3384,7 +3384,7 @@ static int read_blob_object(struct strbuf *buf, const struct object_id *oid, uns
strbuf_addf(buf, "Subproject commit %s\n", oid_to_hex(oid));
} else {
enum object_type type;
unsigned long sz;
size_t sz;
char *result;

result = odb_read_object(the_repository->objects, oid,
Expand Down Expand Up @@ -3611,7 +3611,7 @@ static int load_preimage(struct apply_state *state,

static int resolve_to(struct image *image, const struct object_id *result_id)
{
unsigned long size;
size_t size;
enum object_type type;
char *data;

Expand Down
4 changes: 2 additions & 2 deletions archive.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ static void *object_file_to_archive(const struct archiver_args *args,
const struct object_id *oid,
unsigned int mode,
enum object_type *type,
unsigned long *sizep)
size_t *sizep)
{
void *buffer;
const struct commit *commit = args->convert ? args->commit : NULL;
Expand Down Expand Up @@ -158,7 +158,7 @@ static int write_archive_entry(const struct object_id *oid, const char *base,
write_archive_entry_fn_t write_entry = c->write_entry;
int err;
const char *path_without_prefix;
unsigned long size;
size_t size;
void *buffer;
enum object_type type;

Expand Down
2 changes: 1 addition & 1 deletion attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ static struct attr_stack *read_attr_from_blob(struct index_state *istate,
const char *path, unsigned flags)
{
struct object_id oid;
unsigned long sz;
size_t sz;
enum object_type type;
void *buf;
unsigned short mode;
Expand Down
2 changes: 1 addition & 1 deletion bisect.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ static void show_list(const char *debug, int counted, int nr,
struct commit *commit = p->item;
unsigned commit_flags = commit->object.flags;
enum object_type type;
unsigned long size;
size_t size;
char *buf = odb_read_object(the_repository->objects,
&commit->object.oid, &type,
&size);
Expand Down
15 changes: 11 additions & 4 deletions blame.c
Original file line number Diff line number Diff line change
Expand Up @@ -1041,10 +1041,13 @@ static void fill_origin_blob(struct diff_options *opt,
textconv_object(opt->repo, o->path, o->mode,
&o->blob_oid, 1, &file->ptr, &file_size))
;
else
else {
size_t file_size_st = 0;
file->ptr = odb_read_object(the_repository->objects,
&o->blob_oid, &type,
&file_size);
&file_size_st);
file_size = cast_size_t_to_ulong(file_size_st);
}
file->size = file_size;

if (!file->ptr)
Expand Down Expand Up @@ -2869,10 +2872,14 @@ void setup_scoreboard(struct blame_scoreboard *sb,
textconv_object(sb->repo, sb->path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,
&sb->final_buf_size))
;
else
else {
size_t final_buf_size_st = 0;
sb->final_buf = odb_read_object(the_repository->objects,
&o->blob_oid, &type,
&sb->final_buf_size);
&final_buf_size_st);
sb->final_buf_size =
cast_size_t_to_ulong(final_buf_size_st);
}

if (!sb->final_buf)
die(_("cannot read blob %s for path %s"),
Expand Down
39 changes: 24 additions & 15 deletions builtin/cat-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ static char *replace_idents_using_mailmap(char *object_buf, size_t *size)

static int filter_object(const char *path, unsigned mode,
const struct object_id *oid,
char **buf, unsigned long *size)
char **buf, size_t *size)
{
enum object_type type;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -188,9 +188,15 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
break;

case 'c':
if (textconv_object(the_repository, path, obj_context.mode,
&oid, 1, &buf, &size))
{
unsigned long size_ul = 0;
int textconv_ret = textconv_object(the_repository, path,
obj_context.mode, &oid, 1,
&buf, &size_ul);
size = size_ul;
if (textconv_ret)
break;
}
/* else fallthrough */

case 'p':
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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;
}
Expand All @@ -288,7 +294,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
struct expand_data {
struct object_id oid;
enum object_type type;
unsigned long size;
size_t size;
unsigned short mode;
off_t disk_size;
const char *rest;
Expand Down Expand Up @@ -405,7 +411,7 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
fflush(stdout);
if (opt->transform_mode) {
char *contents;
unsigned long size;
size_t size;

if (!data->rest)
die("missing path for '%s'", oid_to_hex(oid));
Expand All @@ -417,9 +423,12 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
oid_to_hex(oid), data->rest);
} else if (opt->transform_mode == 'c') {
enum object_type type;
if (!textconv_object(the_repository,
data->rest, 0100644, oid,
1, &contents, &size))
unsigned long size_ul = 0;
if (textconv_object(the_repository,
data->rest, 0100644, oid,
1, &contents, &size_ul))
size = size_ul;
else
contents = odb_read_object(the_repository->objects,
oid, &type, &size);
if (!contents)
Expand All @@ -435,7 +444,7 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
}
else {
enum object_type type;
unsigned long size;
size_t size;
void *contents;

contents = odb_read_object(the_repository->objects, oid,
Expand All @@ -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)
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion builtin/difftool.c
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ static char *get_symlink(struct repository *repo,
data = strbuf_detach(&link, NULL);
} else {
enum object_type type;
unsigned long size;
size_t size;
data = odb_read_object(repo->objects, oid, &type, &size);
if (!data)
die(_("could not read object %s for symlink %s"),
Expand Down
7 changes: 5 additions & 2 deletions builtin/fast-export.c
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,10 @@ static void export_blob(const struct object_id *oid)
object = (struct object *)lookup_blob(the_repository, oid);
eaten = 0;
} else {
buf = odb_read_object(the_repository->objects, oid, &type, &size);
size_t size_st = 0;
buf = odb_read_object(the_repository->objects, oid, &type,
&size_st);
size = cast_size_t_to_ulong(size_st);
if (!buf)
die(_("could not read blob %s"), oid_to_hex(oid));
if (check_object_signature(the_repository, oid, buf, size,
Expand Down Expand Up @@ -880,7 +883,7 @@ static char *anonymize_tag(void)

static void handle_tag(const char *name, struct tag *tag)
{
unsigned long size;
size_t size;
enum object_type type;
char *buf;
const char *tagger, *tagger_end, *message;
Expand Down
29 changes: 22 additions & 7 deletions builtin/fast-import.c
Original file line number Diff line number Diff line change
Expand Up @@ -1239,6 +1239,8 @@ static void *gfi_unpack_entry(
unsigned long *sizep)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):

On Thu, Jun 04, 2026 at 10:51:09AM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 82bc6dcc00..3dff898c43 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -1239,6 +1239,8 @@ static void *gfi_unpack_entry(
>  	unsigned long *sizep)
>  {
>  	enum object_type type;
> +	size_t size_st = 0;
> +	void *data;
>  	struct packed_git *p = all_packs[oe->pack_id];
>  	if (p == pack_data && p->pack_size < (pack_size + the_hash_algo->rawsz)) {
>  		/* The object is stored in the packfile we are writing to
> @@ -1260,7 +1262,10 @@ static void *gfi_unpack_entry(
>  		 */
>  		p->pack_size = pack_size + the_hash_algo->rawsz;
>  	}
> -	return unpack_entry(the_repository, p, oe->idx.offset, &type, sizep);
> +	data = unpack_entry(the_repository, p, oe->idx.offset, &type, &size_st);
> +	if (sizep)
> +		*sizep = cast_size_t_to_ulong(size_st);
> +	return data;
>  }

Nit, please feel free to ignore: do we want to add a NEEDSWORK comment
here?

Patrick

{
enum object_type type;
size_t size_st = 0;
void *data;
struct packed_git *p = all_packs[oe->pack_id];
if (p == pack_data && p->pack_size < (pack_size + the_hash_algo->rawsz)) {
/* The object is stored in the packfile we are writing to
Expand All @@ -1260,7 +1262,10 @@ static void *gfi_unpack_entry(
*/
p->pack_size = pack_size + the_hash_algo->rawsz;
}
return unpack_entry(the_repository, p, oe->idx.offset, &type, sizep);
data = unpack_entry(the_repository, p, oe->idx.offset, &type, &size_st);
if (sizep)
*sizep = cast_size_t_to_ulong(size_st);
return data;
}

static void load_tree(struct tree_entry *root)
Expand All @@ -1286,7 +1291,10 @@ static void load_tree(struct tree_entry *root)
die(_("can't load tree %s"), oid_to_hex(oid));
} else {
enum object_type type;
buf = odb_read_object(the_repository->objects, oid, &type, &size);
size_t size_st = 0;
buf = odb_read_object(the_repository->objects, oid, &type,
&size_st);
size = cast_size_t_to_ulong(size_st);
if (!buf || type != OBJ_TREE)
die(_("can't load tree %s"), oid_to_hex(oid));
}
Expand Down Expand Up @@ -2555,7 +2563,7 @@ static void note_change_n(const char *p, struct branch *b, unsigned char *old_fa
die(_("mark :%" PRIuMAX " not a commit"), commit_mark);
oidcpy(&commit_oid, &commit_oe->idx.oid);
} else if (!repo_get_oid(the_repository, p, &commit_oid)) {
unsigned long size;
size_t size;
char *buf = odb_read_object_peeled(the_repository->objects,
&commit_oid, OBJ_COMMIT, &size,
&commit_oid);
Expand Down Expand Up @@ -2622,10 +2630,12 @@ static void parse_from_existing(struct branch *b)
oidclr(&b->branch_tree.versions[1].oid, the_repository->hash_algo);
} else {
unsigned long size;
size_t size_st = 0;
char *buf;

buf = odb_read_object_peeled(the_repository->objects, &b->oid,
OBJ_COMMIT, &size, &b->oid);
OBJ_COMMIT, &size_st, &b->oid);
size = cast_size_t_to_ulong(size_st);
parse_from_commit(b, buf, size);
free(buf);
}
Expand Down Expand Up @@ -2717,7 +2727,7 @@ static struct hash_list *parse_merge(unsigned int *count)
die(_("mark :%" PRIuMAX " not a commit"), idnum);
oidcpy(&n->oid, &oe->idx.oid);
} else if (!repo_get_oid(the_repository, from, &n->oid)) {
unsigned long size;
size_t size;
char *buf = odb_read_object_peeled(the_repository->objects,
&n->oid, OBJ_COMMIT,
&size, &n->oid);
Expand Down Expand Up @@ -3325,7 +3335,10 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid)
char *buf;

if (!oe || oe->pack_id == MAX_PACK_ID) {
buf = odb_read_object(the_repository->objects, oid, &type, &size);
size_t size_st = 0;
buf = odb_read_object(the_repository->objects, oid, &type,
&size_st);
size = cast_size_t_to_ulong(size_st);
} else {
type = oe->type;
buf = gfi_unpack_entry(oe, &size);
Expand Down Expand Up @@ -3433,8 +3446,10 @@ static struct object_entry *dereference(struct object_entry *oe,
buf = gfi_unpack_entry(oe, &size);
} else {
enum object_type unused;
size_t size_st = 0;
buf = odb_read_object(the_repository->objects, oid,
&unused, &size);
&unused, &size_st);
size = cast_size_t_to_ulong(size_st);
}
if (!buf)
die(_("can't load object %s"), oid_to_hex(oid));
Expand Down
2 changes: 1 addition & 1 deletion builtin/fsck.c
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ static int fsck_loose(const struct object_id *oid, const char *path,
struct for_each_loose_cb *data = cb_data;
struct object *obj;
enum object_type type = OBJ_NONE;
unsigned long size;
size_t size;
void *contents = NULL;
int eaten;
struct object_info oi = OBJECT_INFO_INIT;
Expand Down
Loading
Loading