| Message ID | 20250106190855.3098-3-soekkle@freenet.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fixes typemissmatch warinigs from msvc | expand |
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/ > 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.
On Jan 06 2025, Eric Sunshine wrote: > 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/ > >> 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. It can be made more robust by moving the constant to the other side: if (match_string(date, tl->type)+1 >= len) {
diff --git a/date.c b/date.c index a1b26a8dce..17a95077cf 100644 --- a/date.c +++ b/date.c @@ -1244,7 +1244,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm } for (s = special; s->name; s++) { - int len = strlen(s->name); + size_t len = strlen(s->name); if (match_string(date, s->name) == len) { s->fn(tm, now, num); *touched = 1; @@ -1254,7 +1254,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm if (!*num) { for (i = 1; i < 11; i++) { - int len = strlen(number_name[i]); + size_t len = strlen(number_name[i]); if (match_string(date, number_name[i]) == len) { *num = i; *touched = 1; @@ -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) { update_tm(tm, now, tl->length * *num); *num = 0;
Fix compiler warings from msvc in date.c for value truncation from 64 bit to 32 bit integers. 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> --- date.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)