Re: [PATCHv2 2/4] date.c: Fix type missmatch warings from msvc

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 6, 2025, 22:22 UTC
Message-ID: <CAPig+cR0GgZQ+XCyAh=xHRfjfsAdTzC2uyML1vnoM_fXv1Bxew@mail.gmail.com>
In-Reply-To: <20250106190855.3098-3-soekkle@freenet.de>

On Mon, Jan 6, 2025 at 2:14 PM Sören Krecker <soekkle@freenet.de> wrote:

> Fix compiler warings from msvc in date.c for value truncation from 64
> bit to 32 bit integers.

s/warings/warnings/

Show 12 quoted lines

> Also switch from int to size_t for all variables with result of strlen()
> which cannot become negative.
>
> Signed-off-by: Sören Krecker <soekkle@freenet.de>
> ---
> diff --git a/date.c b/date.c
> @@ -1270,7 +1270,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
>         tl = typelen;
>         while (tl->type) {
> -               int len = strlen(tl->type);
> +               size_t len = strlen(tl->type);
>                 if (match_string(date, tl->type) >= len-1) {

This change looks scary and potentially wrong considering that the expression in the `if` statement subtracts 1 from `len`. If `len` happens to be zero, then `len-1` will wrap around to a very large number, thus potentially changing the meaning of the `if` condition.

Now, admittedly, I haven't delved into this code or thought about it much, so I may be entirely wrong about this; perhaps it is impossible for `len` to ever be zero in this context or perhaps the meaning of the `if` condition doesn't change even if it wraps around. But if that's the case, you should use the commit message to explain to readers that you have audited the code and verified that `len` will never be zero or that the condition remains safe despite wraparound. Also, even if you verify that this change is perfectly safe, because it _appears_ to be a potentially behavior breaking change, you should isolate it in its own commit, separate from the other changes, to let reviewers know that it deserves special scrutiny.