Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> I'm afraid I'm still not convinced this is a good idea for the reasons
> I explained previously [1] together with an alternative approach to
> silencing these warnings. What makes "unsigned long" an incorrect
> choice when that's what "git diff" and "git apply" use?
>
> [1]
> https://lore.kernel.org/git/e396131c-1bd3-46d0-bae6-cd97ca9710d8@gmail.com

Ah, this patch still does that?  I was hoping that it got corrected
already after it was pointed out in the previous iterations.  I
agree with you that size_t is a dubious type to use for the line
numbers there.

diff.c defines "struct emit_callback" with lno_in_{pre,post}image
members that are of type "int", which is somewhat dubious, too, but
at least we don't run on 16-bit machines, and being limited to 2
billion lines is probably OK.  I am OK to upgrade that to long (if
we use negative values for some oob signal) or ulong, but that is
clearly outside of this topic.



>> Add macro str_to_size_t for converting a string to size_t.
>> Test if convertion fails with over or underflow.
>
> That is welcome, but the implementation needs tweaking. If you look at
> other uses of strtoul() in our code you'll see that (somewhat
> unusually) one needs to set errno to zero before calling strtoul() as
> one cannot tell from the return value whether there was an error or
> not. As errno may have been set by a previous function call it needs
> to be cleared before calling strtoul() so we can be sure the error
> came from strtoul().

Nice advice.

> Best Wishes
>
> Phillip

Thanks.


By the way, who is
<CAPig+cQ49Hdc_8=mRhhJDTny_Kqo6Wg6Nr98rsBN_YXmBrQ6kA@mail.gmail.com>
and why is such an apparently bogus e-mail address Cc'ed?