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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 6, 2025 at 2:14 PM Sören Krecker <soekkle@xxxxxxxxxx> wrote:
> Fix compiler warings from msvc in date.c for value truncation from 64
> bit to 32 bit integers.

s/warings/warnings/

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





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux