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

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