[PATCHv2 0/4] Fixes typemissmatch warinigs from msvc

[PATCHv2 0/4] Fixes typemissmatch warinigs from msvc

[Sören Krecker]

Thanks for your replays. I try to improve this series and add under and
overflow checks. To improve handling Platform specific stuff I add a macro
for conversion from a string to a size_t.

Best regards

Sören Krecker

Sören Krecker (4):
  add-patch: Fix type missmatch rom msvc
  date.c: Fix type missmatch warings from msvc
  apply.c : Fix type missmatch warings from msvc
  commit.c: Fix type missmatch warings from msvc

 add-patch.c       | 53 +++++++++++++++++++++++++++--------------------
 apply.c           | 37 +++++++++++++++++----------------
 apply.h           |  6 +++---
 commit.c          | 12 +++++------
 date.c            |  6 +++---
 gettext.h         |  2 +-
 git-compat-util.h |  6 ++++++
 7 files changed, 69 insertions(+), 53 deletions(-)


base-commit: 1b4e9a5f8b5f048972c21fe8acafe0404096f694
-- 
2.39.5

[PATCHv2 1/4] add-patch: Fix type missmatch rom msvc

[Sören Krecker]

Fix some compiler warings from msvw in add-patch.c for value truncation
form 64 bit to 32 bit integers.Change unsigned long to size_t for
correct variable size on linux and windows.
Add macro strtos for converting a string to size_t.
Test if convertion fails with over or underflow.

Signed-off-by: Sören Krecker <soekkle@freenet.de>

Uses strtouq

impove linux support

Change Macro name
---
 add-patch.c       | 53 +++++++++++++++++++++++++++--------------------
 gettext.h         |  2 +-
 git-compat-util.h |  6 ++++++
 3 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/add-patch.c b/add-patch.c
index 7b598e14df..67a7f68d23 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -242,7 +242,7 @@ static struct patch_mode patch_mode_worktree_nothead = {
 };
 
 struct hunk_header {
-	unsigned long old_offset, old_count, new_offset, new_count;
+	size_t old_offset, old_count, new_offset, new_count;
 	/*
 	 * Start/end offsets to the extra text after the second `@@` in the
 	 * hunk header, e.g. the function signature. This is expected to
@@ -322,11 +322,12 @@ static void setup_child_process(struct add_p_state *s,
 }
 
 static int parse_range(const char **p,
-		       unsigned long *offset, unsigned long *count)
+		       size_t *offset, size_t *count)
 {
 	char *pend;
-
-	*offset = strtoul(*p, &pend, 10);
+	*offset = strtos(*p, &pend, 10);
+	if (errno == ERANGE)
+		return error("Number dose not fit datatype");
 	if (pend == *p)
 		return -1;
 	if (*pend != ',') {
@@ -334,7 +335,9 @@ static int parse_range(const char **p,
 		*p = pend;
 		return 0;
 	}
-	*count = strtoul(pend + 1, (char **)p, 10);
+	*count = strtos(pend + 1, (char **)p, 10);
+	if (errno == ERANGE)
+		return error("Number dose not fit datatype");
 	return *p == pend + 1 ? -1 : 0;
 }
 
@@ -673,8 +676,8 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
 		 */
 		const char *p;
 		size_t len;
-		unsigned long old_offset = header->old_offset;
-		unsigned long new_offset = header->new_offset;
+		size_t old_offset = header->old_offset;
+		size_t new_offset = header->new_offset;
 
 		if (!colored) {
 			p = s->plain.buf + header->extra_start;
@@ -700,12 +703,14 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
 		else
 			new_offset += delta;
 
-		strbuf_addf(out, "@@ -%lu", old_offset);
+		strbuf_addf(out, "@@ -%" PRIuMAX, (uintmax_t)old_offset);
 		if (header->old_count != 1)
-			strbuf_addf(out, ",%lu", header->old_count);
-		strbuf_addf(out, " +%lu", new_offset);
+			strbuf_addf(out, ",%" PRIuMAX,
+				    (uintmax_t)header->old_count);
+		strbuf_addf(out, " +%" PRIuMAX, (uintmax_t)new_offset);
 		if (header->new_count != 1)
-			strbuf_addf(out, ",%lu", header->new_count);
+			strbuf_addf(out, ",%" PRIuMAX,
+				    (uintmax_t)header->new_count);
 		strbuf_addstr(out, " @@");
 
 		if (len)
@@ -1066,11 +1071,13 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,
 
 	/* last hunk simply gets the rest */
 	if (header->old_offset != remaining.old_offset)
-		BUG("miscounted old_offset: %lu != %lu",
-		    header->old_offset, remaining.old_offset);
+		BUG("miscounted old_offset: %"PRIuMAX" != %"PRIuMAX,
+		    (uintmax_t)header->old_offset,
+		    (uintmax_t)remaining.old_offset);
 	if (header->new_offset != remaining.new_offset)
-		BUG("miscounted new_offset: %lu != %lu",
-		    header->new_offset, remaining.new_offset);
+		BUG("miscounted new_offset: %"PRIuMAX" != %"PRIuMAX,
+		    (uintmax_t)header->new_offset,
+		    (uintmax_t)remaining.new_offset);
 	header->old_count = remaining.old_count;
 	header->new_count = remaining.new_count;
 	hunk->end = end;
@@ -1354,9 +1361,10 @@ static void summarize_hunk(struct add_p_state *s, struct hunk *hunk,
 	struct strbuf *plain = &s->plain;
 	size_t len = out->len, i;
 
-	strbuf_addf(out, " -%lu,%lu +%lu,%lu ",
-		    header->old_offset, header->old_count,
-		    header->new_offset, header->new_count);
+	strbuf_addf(out,
+		    " -%"PRIuMAX",%"PRIuMAX" +%"PRIuMAX",%"PRIuMAX" ",
+		    (uintmax_t)header->old_offset, (uintmax_t)header->old_count,
+		    (uintmax_t)header->new_offset, (uintmax_t)header->new_count);
 	if (out->len - len < SUMMARY_HEADER_WIDTH)
 		strbuf_addchars(out, ' ',
 				SUMMARY_HEADER_WIDTH + len - out->len);
@@ -1625,10 +1633,11 @@ static int patch_update_file(struct add_p_state *s,
 			else if (0 < response && response <= file_diff->hunk_nr)
 				hunk_index = response - 1;
 			else
-				err(s, Q_("Sorry, only %d hunk available.",
-					  "Sorry, only %d hunks available.",
-					  file_diff->hunk_nr),
-				    (int)file_diff->hunk_nr);
+				err(s,
+				    Q_("Sorry, only %"PRIuMAX" hunk available.",
+				       "Sorry, only %"PRIuMAX" hunks available.",
+				       (uintmax_t)file_diff->hunk_nr),
+				    (uintmax_t)file_diff->hunk_nr);
 		} else if (s->answer.buf[0] == '/') {
 			regex_t regex;
 			int ret;
diff --git a/gettext.h b/gettext.h
index 484cafa562..d36f5a7ade 100644
--- a/gettext.h
+++ b/gettext.h
@@ -53,7 +53,7 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
 }
 
 static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
-const char *Q_(const char *msgid, const char *plu, unsigned long n)
+const char *Q_(const char *msgid, const char *plu, size_t n)
 {
 	if (!git_gettext_enabled)
 		return n == 1 ? msgid : plu;
diff --git a/git-compat-util.h b/git-compat-util.h
index e283c46c6f..4c33990a05 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -291,6 +291,12 @@ static inline int _have_unix_sockets(void)
 #ifdef HAVE_BSD_SYSCTL
 #include <sys/sysctl.h>
 #endif
+#if defined _WIN64
+# define strtos strtoull
+#else
+#define strtos strtoul
+#endif
+
 
 /* Used by compat/win32/path-utils.h, and more */
 static inline int is_xplatform_dir_sep(int c)
-- 
2.39.5

Re: [PATCHv2 1/4] add-patch: Fix type missmatch rom msvc

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 7813 bytes --]

On 2025-01-06 at 19:08:52, Sören Krecker wrote:
> Fix some compiler warings from msvw in add-patch.c for value truncation
> form 64 bit to 32 bit integers.Change unsigned long to size_t for
> correct variable size on linux and windows.
> Add macro strtos for converting a string to size_t.
> Test if convertion fails with over or underflow.

A few minor nits here.  We want to say "from" both here and in the title
and "conversion" (and in the title, "mismatch"), and put a space after
the period in a sentence.  I think you meant "MSVC" instead of "msvw",
but if not, please do explain what that is, since I'm not familiar with
it and I'm curious.  The commit message is a good place to explain lots
in detail.

> Signed-off-by: Sören Krecker <soekkle@freenet.de>
> 
> Uses strtouq

I don't see that we're using this function.

> impove linux support
> 
> Change Macro name

We don't typically put comments about the revisions we've made to a
patch in the commit message.  We may put them below the --- so that
they're visible to readers and reviewers, which is helpful, but we
pretend that our patches were perfect to begin with in terms of the
commit message, since the future reader of the history only cares about
the actual end result and not what changes we made along the way.

> ---
>  add-patch.c       | 53 +++++++++++++++++++++++++++--------------------
>  gettext.h         |  2 +-
>  git-compat-util.h |  6 ++++++
>  3 files changed, 38 insertions(+), 23 deletions(-)
> 
> diff --git a/add-patch.c b/add-patch.c
> index 7b598e14df..67a7f68d23 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -242,7 +242,7 @@ static struct patch_mode patch_mode_worktree_nothead = {
>  };
>  
>  struct hunk_header {
> -	unsigned long old_offset, old_count, new_offset, new_count;
> +	size_t old_offset, old_count, new_offset, new_count;
>  	/*
>  	 * Start/end offsets to the extra text after the second `@@` in the
>  	 * hunk header, e.g. the function signature. This is expected to
> @@ -322,11 +322,12 @@ static void setup_child_process(struct add_p_state *s,
>  }
>  
>  static int parse_range(const char **p,
> -		       unsigned long *offset, unsigned long *count)
> +		       size_t *offset, size_t *count)
>  {
>  	char *pend;
> -
> -	*offset = strtoul(*p, &pend, 10);
> +	*offset = strtos(*p, &pend, 10);

I see you've defined this below.

> +	if (errno == ERANGE)
> +		return error("Number dose not fit datatype");

I think the word you wanted was "does".  However, perhaps we should
provide a better, more meaningful error message so the user knows what
data they provided that was invalid.  Maybe "absurdly large value in
diff header range"?  It would be quite bizarre to get a value even as
large as the maximum value of a 32-bit integer, and I don't think our
diff code can even handle values larger than INT_MAX.

In that context, it might not even be necessary to handle values larger
than unsigned long, since we can't generate them.  However, in the
interests of compatibility with other implementations which might not
have that limitation, size_t seems reasonable as a choice to handle more
generally.

Assuming we keep this, we probably also want to mark this for
translation by wrapping it in `_(` and `)`.

I also don't think this order is correct.  In general, errno is not
reset implicitly, so unless we know that an error occurred, errno is
meaningless, since another function could have set it to ERANGE.  We'd
probably need to save errno, set it to 0, and restore to verify that we
got the right value, since we can't distinguish here between a truncated
value for range reasons and for other reasons.

>  	if (pend == *p)
>  		return -1;
>
>  	if (*pend != ',') {
> @@ -334,7 +335,9 @@ static int parse_range(const char **p,
>  		*p = pend;
>  		return 0;
>  	}
> -	*count = strtoul(pend + 1, (char **)p, 10);
> +	*count = strtos(pend + 1, (char **)p, 10);
> +	if (errno == ERANGE)
> +		return error("Number dose not fit datatype");

Same comment here.

>  	return *p == pend + 1 ? -1 : 0;
>  }
>  
> @@ -673,8 +676,8 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
>  		 */
>  		const char *p;
>  		size_t len;
> -		unsigned long old_offset = header->old_offset;
> -		unsigned long new_offset = header->new_offset;
> +		size_t old_offset = header->old_offset;
> +		size_t new_offset = header->new_offset;
>  
>  		if (!colored) {
>  			p = s->plain.buf + header->extra_start;
> @@ -700,12 +703,14 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
>  		else
>  			new_offset += delta;
>  
> -		strbuf_addf(out, "@@ -%lu", old_offset);
> +		strbuf_addf(out, "@@ -%" PRIuMAX, (uintmax_t)old_offset);
>  		if (header->old_count != 1)
> -			strbuf_addf(out, ",%lu", header->old_count);
> -		strbuf_addf(out, " +%lu", new_offset);
> +			strbuf_addf(out, ",%" PRIuMAX,
> +				    (uintmax_t)header->old_count);
> +		strbuf_addf(out, " +%" PRIuMAX, (uintmax_t)new_offset);

If we're using size_t, we can use %zu.  That's specified in C99 as the
appropriate formatting type for size_t, and we require C99 or C11 for
all systems.  We don't need to cast to uintmax_t.

> diff --git a/gettext.h b/gettext.h
> index 484cafa562..d36f5a7ade 100644
> --- a/gettext.h
> +++ b/gettext.h
> @@ -53,7 +53,7 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
>  }
>  
>  static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
> -const char *Q_(const char *msgid, const char *plu, unsigned long n)
> +const char *Q_(const char *msgid, const char *plu, size_t n)
>  {
>  	if (!git_gettext_enabled)
>  		return n == 1 ? msgid : plu;
> diff --git a/git-compat-util.h b/git-compat-util.h
> index e283c46c6f..4c33990a05 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -291,6 +291,12 @@ static inline int _have_unix_sockets(void)
>  #ifdef HAVE_BSD_SYSCTL
>  #include <sys/sysctl.h>
>  #endif
> +#if defined _WIN64
> +# define strtos strtoull
> +#else
> +#define strtos strtoul
> +#endif

This is not a great name for the function.  First of all, it resembles
the standard functions a lot, so it's something that POSIX could
standardize or an OS could add, and then we'll have some fun compilation
errors when we redefine things.

Second, it's a lot less future-proof.  While I do agree that only
Windows 64-bit systems are likely to fall into this case, since we
already include <limits.h>, we probably should do this:

  #if SIZE_MAX == ULONG_MAX
  #define str_to_size_t strtoul
  #else
  #define str_to_size_t strtoull
  #endif

(or whatever you want to call the function).

That expresses what we care about—that the type is suitable for the
value we want to parse—and doesn't use the OS as a proxy for that data.
Otherwise, the Unix developer who doesn't use Windows may not
understand _why_ Windows is special and the reason we've chosen this
change.

On that note, it would be helpful if you explained in the commit message
why that is for people who don't know.  Maybe something like this:

  On 64-bit systems, size_t is a 64-bit type.  On most Unix systems,
  unsigned long is also 64 bits in size, so we can use functions for
  that type to parse values of size_t.  However, on Windows, unsigned
  long is always 32 bits, and if we want a 64-bit type, we must use
  unsigned long long.  To future-proof our changes against other
  platforms that might be added in the future, we first check if
  unsigned long is sufficient, and otherwise, use unsigned long long,
  which will work in both cases.

Of course, please feel free to edit as you see fit.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCHv2 1/4] add-patch: Fix type missmatch rom msvc

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> We don't typically put comments about the revisions we've made to a
> patch in the commit message.  We may put them below the --- so that
> they're visible to readers and reviewers, which is helpful, but we
> pretend that our patches were perfect to begin with in terms of the
> commit message, since the future reader of the history only cares about
> the actual end result and not what changes we made along the way.

Thanks, all true.  The future readers would only *see* the end
results, and we do not want to hear about the previous stumblings
the author made before reaching an acceptable version.

>> ---
>>  add-patch.c       | 53 +++++++++++++++++++++++++++--------------------
>>  gettext.h         |  2 +-
>>  git-compat-util.h |  6 ++++++
>>  3 files changed, 38 insertions(+), 23 deletions(-)

I already made this comment, but I think the offset/count being
ulong is a very sane design decision, and what is causing the
compiler warning is some earlier change that introduced size_t
variables or parameters in the callchain.  As far as I can tell,
there is no system functions that yields size_t (hence we must use
size_t everywhere) in the code paths that deal with offset and
count.  I suggested to find these abused size_t and fix them to use
the matching type, i.e. "unsigned long", instead, as an alternative
fix.  I did not get an impression that the author tried the approach
and found why we must use size_t for offset/count instead.

And if we go that route, there is *no* need to talk about 64-bit ve
32-bit platforms.  ulong used consistently everywhere would let you
use offset/count that fits in ulong, and the apply machinery is
artificially limited to limit the patch size to a few gigabytes, so
32-bit ulong should be plenty as Phillip pointed out earlier.

If we needed to parse an integer into a large integer, the existing
code seem to use strtoumax() into uintmax_t and move it to the
target (while checking for truncation).  "Ah we are on windows, so
use strtoll, otherwise use strtol" is not something we want to see
in our codebase.

> If we're using size_t, we can use %zu.  That's specified in C99 as the
> appropriate formatting type for size_t, and we require C99 or C11 for
> all systems.  We don't need to cast to uintmax_t.

You and Documentation/CodingGuidelines contradict with each other
here.

Thanks for a review.

Re: [PATCHv2 1/4] add-patch: Fix type missmatch rom msvc

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> If we're using size_t, we can use %zu.  That's specified in C99 as the
>> appropriate formatting type for size_t, and we require C99 or C11 for
>> all systems.  We don't need to cast to uintmax_t.
>
> You and Documentation/CodingGuidelines contradict with each other
> here.

By this, I do not necessarily mean that we should stick to the past
tradition since d7d850e2 (CodingGuidelines: mention C99 features we
can't use, 2022-10-10), written back when MSVC was claiming to do
C99 without letting us use %z conversion.

What I meant was that if we are to update our stance against %z
conversion after re-evaluating the situation (and such time will
certainly come someday---I do not offhand know if it can be today),
we should update the documentation before or at least at the same
time we recommend its use to new people.

Thanks.

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

[Sören Krecker]

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(-)

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;
-- 
2.39.5

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

[Eric Sunshine]

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.

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

[Andreas Schwab]

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) {

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

[PATCHv2 3/4] apply.c : Fix type missmatch warings from msvc

[Sören Krecker]

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>
---
 apply.c | 37 +++++++++++++++++++------------------
 apply.h |  6 +++---
 2 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/apply.c b/apply.c
index 4a7b6120ac..b896889505 100644
--- a/apply.c
+++ b/apply.c
@@ -414,9 +414,9 @@ static int read_patch_file(struct strbuf *sb, int fd)
 	return 0;
 }
 
-static unsigned long linelen(const char *buffer, unsigned long size)
+static size_t linelen(const char *buffer, size_t size)
 {
-	unsigned long len = 0;
+	size_t len = 0;
 	while (size--) {
 		len++;
 		if (*buffer++ == '\n')
@@ -688,7 +688,7 @@ static char *find_name_common(struct strbuf *root,
 	 * or "file~").
 	 */
 	if (def) {
-		int deflen = strlen(def);
+		size_t deflen = strlen(def);
 		if (deflen < len && !strncmp(start, def, deflen))
 			return squash_slash(xstrdup(def));
 	}
@@ -1088,7 +1088,7 @@ static int gitdiff_index(struct gitdiff_data *state,
 	 */
 	const char *ptr, *eol;
 	int len;
-	const unsigned hexsz = the_hash_algo->hexsz;
+	const size_t hexsz = the_hash_algo->hexsz;
 
 	ptr = strchr(line, '.');
 	if (!ptr || ptr[1] != '.' || hexsz < ptr - line)
@@ -1131,7 +1131,7 @@ static int gitdiff_unrecognized(struct gitdiff_data *state UNUSED,
  */
 static const char *skip_tree_prefix(int p_value,
 				    const char *line,
-				    int llen)
+				    size_t llen)
 {
 	int nslash;
 	int i;
@@ -1158,7 +1158,7 @@ static const char *skip_tree_prefix(int p_value,
  */
 static char *git_header_name(int p_value,
 			     const char *line,
-			     int llen)
+			     ssize_t llen)
 {
 	const char *name;
 	const char *second = NULL;
@@ -1313,15 +1313,15 @@ static int check_header_line(int linenr, struct patch *patch)
 	return 0;
 }
 
-int parse_git_diff_header(struct strbuf *root,
+size_t parse_git_diff_header(struct strbuf *root,
 			  int *linenr,
 			  int p_value,
 			  const char *line,
-			  int len,
-			  unsigned int size,
+			  size_t len,
+			  size_t size,
 			  struct patch *patch)
 {
-	unsigned long offset;
+	size_t offset;
 	struct gitdiff_data parse_hdr_state;
 
 	/* A git diff has explicit new/delete information, so we don't guess */
@@ -1378,7 +1378,7 @@ int parse_git_diff_header(struct strbuf *root,
 			break;
 		for (i = 0; i < ARRAY_SIZE(optable); i++) {
 			const struct opentry *p = optable + i;
-			int oplen = strlen(p->str);
+			size_t oplen = strlen(p->str);
 			int res;
 			if (len < oplen || memcmp(p->str, line, oplen))
 				continue;
@@ -1430,7 +1430,8 @@ static int parse_num(const char *line, unsigned long *p)
 static int parse_range(const char *line, int len, int offset, const char *expect,
 		       unsigned long *p1, unsigned long *p2)
 {
-	int digits, ex;
+	int digits;
+	size_t ex;
 
 	if (offset < 0 || offset >= len)
 		return -1;
@@ -1465,7 +1466,7 @@ static int parse_range(const char *line, int len, int offset, const char *expect
 	return offset + ex;
 }
 
-static void recount_diff(const char *line, int size, struct fragment *fragment)
+static void recount_diff(const char *line, size_t size, struct fragment *fragment)
 {
 	int oldlines = 0, newlines = 0, ret = 0;
 
@@ -1475,7 +1476,7 @@ static void recount_diff(const char *line, int size, struct fragment *fragment)
 	}
 
 	for (;;) {
-		int len = linelen(line, size);
+		size_t len = linelen(line, size);
 		size -= len;
 		line += len;
 
@@ -1543,11 +1544,11 @@ static int parse_fragment_header(const char *line, int len, struct fragment *fra
  */
 static int find_header(struct apply_state *state,
 		       const char *line,
-		       unsigned long size,
+		       size_t size,
 		       int *hdrsize,
 		       struct patch *patch)
 {
-	unsigned long offset, len;
+	size_t offset, len;
 
 	patch->is_toplevel_relative = 0;
 	patch->is_rename = patch->is_copy = 0;
@@ -2132,7 +2133,7 @@ static int use_patch(struct apply_state *state, struct patch *p)
  *   the number of bytes consumed otherwise,
  *     so that the caller can call us again for the next patch.
  */
-static int parse_chunk(struct apply_state *state, char *buffer, unsigned long size, struct patch *patch)
+static int parse_chunk(struct apply_state *state, char *buffer, size_t size, struct patch *patch)
 {
 	int hdrsize, patchsize;
 	int offset = find_header(state, buffer, size, &hdrsize, patch);
@@ -2491,7 +2492,7 @@ static int match_fragment(struct apply_state *state,
 	struct strbuf fixed = STRBUF_INIT;
 	char *fixed_buf;
 	size_t fixed_len;
-	int preimage_limit;
+	ssize_t preimage_limit;
 	int ret;
 
 	if (preimage->line_nr + current_lno <= img->line_nr) {
diff --git a/apply.h b/apply.h
index 90e887ec0e..bb01ce7dbc 100644
--- a/apply.h
+++ b/apply.h
@@ -166,12 +166,12 @@ int check_apply_state(struct apply_state *state, int force_apply);
  *
  * Returns -1 on failure, the length of the parsed header otherwise.
  */
-int parse_git_diff_header(struct strbuf *root,
+size_t parse_git_diff_header(struct strbuf *root,
 			  int *linenr,
 			  int p_value,
 			  const char *line,
-			  int len,
-			  unsigned int size,
+			  size_t len,
+			  size_t size,
 			  struct patch *patch);
 
 void release_patch(struct patch *patch);
-- 
2.39.5

Re: [PATCHv2 3/4] apply.c : Fix type missmatch warings from msvc

[Eric Sunshine]

On Mon, Jan 6, 2025 at 2:15 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/
s/date.c/apply.c/

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

[PATCHv2 4/4] commit.c: Fix type missmatch warings from msvc

[Sören Krecker]

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>
---
 commit.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/commit.c b/commit.c
index a127fe60c5..78993395e6 100644
--- a/commit.c
+++ b/commit.c
@@ -466,8 +466,8 @@ int parse_commit_buffer(struct repository *r, struct commit *item, const void *b
 	struct object_id parent;
 	struct commit_list **pptr;
 	struct commit_graft *graft;
-	const int tree_entry_len = the_hash_algo->hexsz + 5;
-	const int parent_entry_len = the_hash_algo->hexsz + 7;
+	const size_t tree_entry_len = the_hash_algo->hexsz + 5;
+	const size_t parent_entry_len = the_hash_algo->hexsz + 7;
 	struct tree *tree;
 
 	if (item->object.parsed)
@@ -1114,10 +1114,10 @@ static const char *gpg_sig_headers[] = {
 
 int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct git_hash_algo *algo)
 {
-	int inspos, copypos;
+	ssize_t inspos, copypos;
 	const char *eoh;
 	const char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algo)];
-	int gpg_sig_header_len = strlen(gpg_sig_header);
+	size_t gpg_sig_header_len = strlen(gpg_sig_header);
 
 	/* find the end of the header */
 	eoh = strstr(buf->buf, "\n\n");
@@ -1530,7 +1530,7 @@ int commit_tree(const char *msg, size_t msg_len, const struct object_id *tree,
 	return result;
 }
 
-static int find_invalid_utf8(const char *buf, int len)
+static int find_invalid_utf8(const char *buf, size_t len)
 {
 	int offset = 0;
 	static const unsigned int max_codepoint[] = {
@@ -1539,7 +1539,7 @@ static int find_invalid_utf8(const char *buf, int len)
 
 	while (len) {
 		unsigned char c = *buf++;
-		int bytes, bad_offset;
+		size_t bytes, bad_offset;
 		unsigned int codepoint;
 		unsigned int min_val, max_val;
 
-- 
2.39.5

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

[Eric Sunshine]

On Mon, Jan 6, 2025 at 2:15 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/
s/date.c/commit.c/

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