Re: [PATCH v4 2/4] add-patch: modify patch_update_file() signature

[Junio C Hamano <gitster@pobox.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 ...

>  {
>  	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.