Re: [PATCH v4 2/4] add-patch: modify patch_update_file() signature
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 13, 2026, 23:33 UTC
- Message-ID
- <xmqqms1ci5g8.fsf@gitster.g>
- In-Reply-To
- <906f25e184d744f9d23681600a0d9e440b7f07df.1771015581.git.abrahamadekunle50@gmail.com>
Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> writes:
> -static int patch_update_file(struct add_p_state *s, > - struct file_diff *file_diff) > +static ssize_t patch_update_file(struct add_p_state *s, size_t idx)
Why ssize_t? Are we going to handle that many hunks that we do not expect to fit in a platform natural "int" type? If we are not doing anything about "idx" being more than half the type, which apparently is the case ...
Show 11 quoted lines
> {
> size_t hunk_index = 0;
> ssize_t i, undecided_previous, undecided_next, rendered_hunk_index = -1;
> struct hunk *hunk;
> char ch;
> struct child_process cp = CHILD_PROCESS_INIT;
> - int colored = !!s->colored.len, quit = 0, use_pager = 0;
> + int colored = !!s->colored.len, use_pager = 0;
> enum prompt_mode_type prompt_mode_type;
> + struct file_diff *file_diff = s->file_diff + idx;
> + ssize_t patch_update_resp = (ssize_t)idx;... with the cast that is not checked here, wouldn't it make sense to just use the platform natural "int" everywhere? Your code is not "safe" either way. I do not think we expect to handle 2 billion hunks, so even on 32-bit platforms, platform natural "int" should be plenty. Instead of religiously using size_t and ssize_t to count things without extra care, I'd rather see us check the error condition for real, if that is what we really care about (and that can still be done while leaving the codebase cleaner by sticking to the platform natural "int").
Enough ranting. Anyway.
If we really are bothered that we cannot handle 3 billion hunks, we could avoid losing half the number range by returning s->file_diff.file_diff_nr (which is one more than there are elements in s->file_diff[] array) or ((size_t)-1). That would allow us to return size_t from here. I care about this a bit more than "why use size_t when int is perfectly fine", because some platforms that are not quite POSIX can have ssize_t that is not as wide as size_t.