{"thread":{"id":"62857","subject":"[PATCH v3 0/4] Fix type conversion Warings from msvc","startedAt":"2025-01-26T13:02:09Z","lastAt":"2025-01-30T19:24:23Z","messageCount":9,"participants":["Sören Krecker","Patrick Steinhardt","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"511210","messageId":"20250126125638.3089-1-soekkle@freenet.de","threadId":"62857","inReplyTo":null,"subject":"[PATCH v3 0/4] Fix type conversion Warings from msvc","fromName":"Sören Krecker","fromEmail":"soekkle@freenet.de","sentAt":"2025-01-26T12:56:34Z","receivedAt":"2025-01-26T13:02:09Z","isPatch":true,"sender":{"key":"soekkle@freenet.de","avatar":"https://avatars.githubusercontent.com/u/6253399?v=4"},"body":"Hi everyone,\nsorry for my late reply and thanks for your suggestions.\nI am trying to improve this patch series.\n\nSören Krecker (4):\n add-patch: Fix type conversion warnings from msvc\n date.c: Fix type conversation warnings from msvc\n apply.c : Fix type conversation warnings from msvc\n commit.c: Fix type conversation warnings from msvc\n\n add-patch.c | 53 +++++++++++++++++++++++++++--------------------\n apply.c | 37 +++++++++++++++++----------------\n apply.h | 6 +++---\n commit.c | 12 +++++------\n date.c | 8 +++----\n gettext.h | 2 +-\n git-compat-util.h | 7 +++++++\n 7 files changed, 71 insertions(+), 54 deletions(-)\n\n\nbase-commit: 5f8f7081f7761acdf83d0a4c6819fe3d724f01d7\n-- \n2.39.5\n\n"},{"id":"511211","messageId":"20250126125638.3089-2-soekkle@freenet.de","threadId":"62857","inReplyTo":"20250126125638.3089-1-soekkle@freenet.de","subject":"[PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc","fromName":"Sören Krecker","fromEmail":"soekkle@freenet.de","sentAt":"2025-01-26T12:56:35Z","receivedAt":"2025-01-26T13:02:23Z","isPatch":true,"sender":{"key":"soekkle@freenet.de","avatar":"https://avatars.githubusercontent.com/u/6253399?v=4"},"body":"Fix some compiler warnings from msvc in add-patch.c for value truncation\nform 64 bit to 32 bit integers. Change unsigned long to size_t for\ncorrect variable size on linux and windows.\nAdd macro str_to_size_t for converting a string to size_t.\nTest if convertion fails with over or underflow.\n\nSigned-off-by: Sören Krecker \n---\n add-patch.c | 53 +++++++++++++++++++++++++++--------------------\n gettext.h | 2 +-\n git-compat-util.h | 7 +++++++\n 3 files changed, 39 insertions(+), 23 deletions(-)\n\ndiff --git a/add-patch.c b/add-patch.c\nindex 95c67d8c80..4fb6ae2c4b 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -242,7 +242,7 @@ static struct patch_mode patch_mode_worktree_nothead = {\n };\n \n struct hunk_header {\n-\tunsigned long old_offset, old_count, new_offset, new_count;\n+\tsize_t old_offset, old_count, new_offset, new_count;\n \t/*\n \t * Start/end offsets to the extra text after the second `@@` in the\n \t * hunk header, e.g. the function signature. This is expected to\n@@ -322,11 +322,12 @@ static void setup_child_process(struct add_p_state *s,\n }\n \n static int parse_range(const char **p,\n-\t\t unsigned long *offset, unsigned long *count)\n+\t\t size_t *offset, size_t *count)\n {\n \tchar *pend;\n-\n-\t*offset = strtoul(*p, &pend, 10);\n+\t*offset = str_to_size_t(*p, &pend, 10);\n+\tif (errno == ERANGE)\n+\t\treturn error(_(\"Number is too large for this field\"));\n \tif (pend == *p)\n \t\treturn -1;\n \tif (*pend != ',') {\n@@ -334,7 +335,9 @@ static int parse_range(const char **p,\n \t\t*p = pend;\n \t\treturn 0;\n \t}\n-\t*count = strtoul(pend + 1, (char **)p, 10);\n+\t*count = str_to_size_t(pend + 1, (char **)p, 10);\n+\tif (errno == ERANGE)\n+\t\treturn error(_(\"Number is too large for this field\"));\n \treturn *p == pend + 1 ? -1 : 0;\n }\n \n@@ -673,8 +676,8 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,\n \t\t */\n \t\tconst char *p;\n \t\tsize_t len;\n-\t\tunsigned long old_offset = header->old_offset;\n-\t\tunsigned long new_offset = header->new_offset;\n+\t\tsize_t old_offset = header->old_offset;\n+\t\tsize_t new_offset = header->new_offset;\n \n \t\tif (!colored) {\n \t\t\tp = s->plain.buf + header->extra_start;\n@@ -700,12 +703,14 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,\n \t\telse\n \t\t\tnew_offset += delta;\n \n-\t\tstrbuf_addf(out, \"@@ -%lu\", old_offset);\n+\t\tstrbuf_addf(out, \"@@ -%\" PRIuMAX, (uintmax_t)old_offset);\n \t\tif (header->old_count != 1)\n-\t\t\tstrbuf_addf(out, \",%lu\", header->old_count);\n-\t\tstrbuf_addf(out, \" +%lu\", new_offset);\n+\t\t\tstrbuf_addf(out, \",%\" PRIuMAX,\n+\t\t\t\t (uintmax_t)header->old_count);\n+\t\tstrbuf_addf(out, \" +%\" PRIuMAX, (uintmax_t)new_offset);\n \t\tif (header->new_count != 1)\n-\t\t\tstrbuf_addf(out, \",%lu\", header->new_count);\n+\t\t\tstrbuf_addf(out, \",%\" PRIuMAX,\n+\t\t\t\t (uintmax_t)header->new_count);\n \t\tstrbuf_addstr(out, \" @@\");\n \n \t\tif (len)\n@@ -1066,11 +1071,13 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,\n \n \t/* last hunk simply gets the rest */\n \tif (header->old_offset != remaining.old_offset)\n-\t\tBUG(\"miscounted old_offset: %lu != %lu\",\n-\t\t header->old_offset, remaining.old_offset);\n+\t\tBUG(\"miscounted old_offset: %\"PRIuMAX\" != %\"PRIuMAX,\n+\t\t (uintmax_t)header->old_offset,\n+\t\t (uintmax_t)remaining.old_offset);\n \tif (header->new_offset != remaining.new_offset)\n-\t\tBUG(\"miscounted new_offset: %lu != %lu\",\n-\t\t header->new_offset, remaining.new_offset);\n+\t\tBUG(\"miscounted new_offset: %\"PRIuMAX\" != %\"PRIuMAX,\n+\t\t (uintmax_t)header->new_offset,\n+\t\t (uintmax_t)remaining.new_offset);\n \theader->old_count = remaining.old_count;\n \theader->new_count = remaining.new_count;\n \thunk->end = end;\n@@ -1354,9 +1361,10 @@ static void summarize_hunk(struct add_p_state *s, struct hunk *hunk,\n \tstruct strbuf *plain = &s->plain;\n \tsize_t len = out->len, i;\n \n-\tstrbuf_addf(out, \" -%lu,%lu +%lu,%lu \",\n-\t\t header->old_offset, header->old_count,\n-\t\t header->new_offset, header->new_count);\n+\tstrbuf_addf(out,\n+\t\t \" -%\"PRIuMAX\",%\"PRIuMAX\" +%\"PRIuMAX\",%\"PRIuMAX\" \",\n+\t\t (uintmax_t)header->old_offset, (uintmax_t)header->old_count,\n+\t\t (uintmax_t)header->new_offset, (uintmax_t)header->new_count);\n \tif (out->len - len < SUMMARY_HEADER_WIDTH)\n \t\tstrbuf_addchars(out, ' ',\n \t\t\t\tSUMMARY_HEADER_WIDTH + len - out->len);\n@@ -1625,10 +1633,11 @@ static int patch_update_file(struct add_p_state *s,\n \t\t\telse if (0 < response && response <= file_diff->hunk_nr)\n \t\t\t\thunk_index = response - 1;\n \t\t\telse\n-\t\t\t\terr(s, Q_(\"Sorry, only %d hunk available.\",\n-\t\t\t\t\t \"Sorry, only %d hunks available.\",\n-\t\t\t\t\t file_diff->hunk_nr),\n-\t\t\t\t (int)file_diff->hunk_nr);\n+\t\t\t\terr(s,\n+\t\t\t\t Q_(\"Sorry, only %\"PRIuMAX\" hunk available.\",\n+\t\t\t\t \"Sorry, only %\"PRIuMAX\" hunks available.\",\n+\t\t\t\t (uintmax_t)file_diff->hunk_nr),\n+\t\t\t\t (uintmax_t)file_diff->hunk_nr);\n \t\t} else if (s->answer.buf[0] == '/') {\n \t\t\tregex_t regex;\n \t\t\tint ret;\ndiff --git a/gettext.h b/gettext.h\nindex 484cafa562..d36f5a7ade 100644\n--- a/gettext.h\n+++ b/gettext.h\n@@ -53,7 +53,7 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)\n }\n \n static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)\n-const char *Q_(const char *msgid, const char *plu, unsigned long n)\n+const char *Q_(const char *msgid, const char *plu, size_t n)\n {\n \tif (!git_gettext_enabled)\n \t\treturn n == 1 ? msgid : plu;\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex e283c46c6f..bb9a6c2bc4 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -292,6 +292,13 @@ static inline int _have_unix_sockets(void)\n #include \n #endif\n \n+#if SIZE_MAX == ULONG_MAX\n+#define str_to_size_t strtoul\n+#else\n+#define str_to_size_t strtoull\n+#endif\n+\n+\n /* Used by compat/win32/path-utils.h, and more */\n static inline int is_xplatform_dir_sep(int c)\n {\n-- \n2.39.5\n\n"},{"id":"511223","messageId":"Z5c1EIXi7nsB2kJe@pks.im","threadId":"62857","inReplyTo":"20250126125638.3089-2-soekkle@freenet.de","subject":"Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-27T07:26:08Z","receivedAt":"2025-01-27T07:26:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Note: the word after the subject's subsystem should start with a\nlower-case letter.\n\nOn Sun, Jan 26, 2025 at 01:56:35PM +0100, Sören Krecker wrote:\n> Fix some compiler warnings from msvc in add-patch.c for value truncation\n> form 64 bit to 32 bit integers. Change unsigned long to size_t for\n> correct variable size on linux and windows.\n> Add macro str_to_size_t for converting a string to size_t.\n\nThere shouldn't be a need for this macro, we already have `strtoumax()`.\nAnd in case the platform doesn't provide it we know to provide our own\nimplementation.\n\n> Test if convertion fails with over or underflow.\n\ns/convertion/conversion/\n\n> diff --git a/add-patch.c b/add-patch.c\n> index 95c67d8c80..4fb6ae2c4b 100644\n> --- a/add-patch.c\n> +++ b/add-patch.c\n> @@ -322,11 +322,12 @@ static void setup_child_process(struct add_p_state *s,\n> }\n> \n> static int parse_range(const char **p,\n> -\t\t unsigned long *offset, unsigned long *count)\n> +\t\t size_t *offset, size_t *count)\n> {\n> \tchar *pend;\n> -\n> -\t*offset = strtoul(*p, &pend, 10);\n> +\t*offset = str_to_size_t(*p, &pend, 10);\n> +\tif (errno == ERANGE)\n> +\t\treturn error(_(\"Number is too large for this field\"));\n\nError messages should start with a lower-case letter.\n\n> \tif (pend == *p)\n> \t\treturn -1;\n> \tif (*pend != ',') {\n> @@ -334,7 +335,9 @@ static int parse_range(const char **p,\n> \t\t*p = pend;\n> \t\treturn 0;\n> \t}\n> -\t*count = strtoul(pend + 1, (char **)p, 10);\n> +\t*count = str_to_size_t(pend + 1, (char **)p, 10);\n> +\tif (errno == ERANGE)\n> +\t\treturn error(_(\"Number is too large for this field\"));\n\nHere, too.\n\n> @@ -1066,11 +1071,13 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,\n> \n> \t/* last hunk simply gets the rest */\n> \tif (header->old_offset != remaining.old_offset)\n> -\t\tBUG(\"miscounted old_offset: %lu != %lu\",\n> -\t\t header->old_offset, remaining.old_offset);\n> +\t\tBUG(\"miscounted old_offset: %\"PRIuMAX\" != %\"PRIuMAX,\n> +\t\t (uintmax_t)header->old_offset,\n> +\t\t (uintmax_t)remaining.old_offset);\n> \tif (header->new_offset != remaining.new_offset)\n> -\t\tBUG(\"miscounted new_offset: %lu != %lu\",\n> -\t\t header->new_offset, remaining.new_offset);\n> +\t\tBUG(\"miscounted new_offset: %\"PRIuMAX\" != %\"PRIuMAX,\n> +\t\t (uintmax_t)header->new_offset,\n> +\t\t (uintmax_t)remaining.new_offset);\n> \theader->old_count = remaining.old_count;\n> \theader->new_count = remaining.new_count;\n> \thunk->end = end;\n\nI feel like most of the changes are adapting formatting directives like\nthis. Might be worthwhile to separate into a standalone commit. That'd\nalso allow the commit message to read less like a list of bullet points\nand provide more context, explaining the actual change.\n\n> diff --git a/gettext.h b/gettext.h\n> index 484cafa562..d36f5a7ade 100644\n> --- a/gettext.h\n> +++ b/gettext.h\n> @@ -53,7 +53,7 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)\n> }\n> \n> static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)\n> -const char *Q_(const char *msgid, const char *plu, unsigned long n)\n> +const char *Q_(const char *msgid, const char *plu, size_t n)\n> {\n> \tif (!git_gettext_enabled)\n> \t\treturn n == 1 ? msgid : plu;\n\nThis change feels completely unrelated to all the other changes. It\nwould probably warrant a new commit.\n\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index e283c46c6f..bb9a6c2bc4 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -292,6 +292,13 @@ static inline int _have_unix_sockets(void)\n> #include \n> #endif\n> \n> +#if SIZE_MAX == ULONG_MAX\n> +#define str_to_size_t strtoul\n> +#else\n> +#define str_to_size_t strtoull\n> +#endif\n\nHm. A couple of comments:\n\n - The function name doesn't match the schema of function names we\n already have. I would rather have expected it to be called something\n like `strtouz()` or something like that.\n\n - We tend to avoid using `strtoul()` and friends directly, as they are\n really hard to get right. See the implementation of `strtoul_ui()`\n for all the checks we do there.\n\n - The way the macro is implemented feels quite fragile.\n\nSo I'd propose to adapt the approach a bit and introduce a new function\n`strtoumax_ui()`:\n\n static inline int strtoumax_ui(char *const *s, int base, unsigned\n uintmax_t max, int *result);\n\nThe implementation would mostly follow what we have in `strotul_ui()`.\nThe `max` parameter here could be used to control the maximum that the\ncaller expects -- if the parsed integer exceeds it, it would return an\nerror and set `ERANGE`. If we had such a helper, we can then also\nreimplement `strtoul_ui()` on top of that function with a simple call to\n`strtoumax_ui(s, base, UINT_MAX, result)`.\n\nThis would overall be a lot more flexible than what we currently have.\n\nPatrick\n"},{"id":"511224","messageId":"Z5c1HdBheaiA87QH@pks.im","threadId":"62857","inReplyTo":"20250126125638.3089-1-soekkle@freenet.de","subject":"Re: [PATCH v3 0/4] Fix type conversion Warings from msvc","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-27T07:26:21Z","receivedAt":"2025-01-27T07:26:27Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Jan 26, 2025 at 01:56:34PM +0100, Sören Krecker wrote:\n> Hi everyone,\n> sorry for my late reply and thanks for your suggestions.\n> I am trying to improve this patch series.\n\nThanks for rerolling! I've got a couple more comments.\n\nSomething seems to have gone wrong when sending your patches. Only the\nfirst patch is connected to the cover letter, the remaining ones aren't.\n\nPatrick\n"},{"id":"511277","messageId":"xmqq1pwow83z.fsf@gitster.g","threadId":"62857","inReplyTo":"Z5c1EIXi7nsB2kJe@pks.im","subject":"Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-27T16:10:24Z","receivedAt":"2025-01-27T16:10:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Note: the word after the subject's subsystem should start with a\n> lower-case letter.\n>\n> On Sun, Jan 26, 2025 at 01:56:35PM +0100, Sören Krecker wrote:\n>> Fix some compiler warnings from msvc in add-patch.c for value truncation\n>> form 64 bit to 32 bit integers. Change unsigned long to size_t for\n>> correct variable size on linux and windows.\n>> Add macro str_to_size_t for converting a string to size_t.\n>\n> There shouldn't be a need for this macro, we already have `strtoumax()`.\n> And in case the platform doesn't provide it we know to provide our own\n> implementation.\n\nThanks for a detailed review; I'll omit them as I agree with all you\nsaid there.\n\nIf I pretend for a while that moving from ulong to size_t is a good\nchange for line numbers and line counts in the first place, that is.\n\nIn other words, I agree with all the improvements your comments\nsuggest to the _implementation_.\n\nThanks.\n"},{"id":"511420","messageId":"6a251603-25bc-415d-ab8c-ae698bd7977a@gmail.com","threadId":"62857","inReplyTo":"20250126125638.3089-2-soekkle@freenet.de","subject":"Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-01-29T16:51:42Z","receivedAt":"2025-01-29T16:51:47Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Sören\n\nOn 26/01/2025 12:56, Sören Krecker wrote:\n> Fix some compiler warnings from msvc in add-patch.c for value truncation\n> form 64 bit to 32 bit integers. Change unsigned long to size_t for\n> correct variable size on linux and windows.\n\nI'm afraid I'm still not convinced this is a good idea for the reasons I \nexplained previously [1] together with an alternative approach to \nsilencing these warnings. What makes \"unsigned long\" an incorrect choice \nwhen that's what \"git diff\" and \"git apply\" use?\n\n[1] \nhttps://lore.kernel.org/git/e396131c-1bd3-46d0-bae6-cd97ca9710d8@gmail.com\n\n> Add macro str_to_size_t for converting a string to size_t.\n> Test if convertion fails with over or underflow.\n\nThat is welcome, but the implementation needs tweaking. If you look at \nother uses of strtoul() in our code you'll see that (somewhat unusually) \none needs to set errno to zero before calling strtoul() as one cannot \ntell from the return value whether there was an error or not. As errno \nmay have been set by a previous function call it needs to be cleared \nbefore calling strtoul() so we can be sure the error came from strtoul().\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Sören Krecker \n> ---\n> add-patch.c | 53 +++++++++++++++++++++++++++--------------------\n> gettext.h | 2 +-\n> git-compat-util.h | 7 +++++++\n> 3 files changed, 39 insertions(+), 23 deletions(-)\n> \n> diff --git a/add-patch.c b/add-patch.c\n> index 95c67d8c80..4fb6ae2c4b 100644\n> --- a/add-patch.c\n> +++ b/add-patch.c\n> @@ -242,7 +242,7 @@ static struct patch_mode patch_mode_worktree_nothead = {\n> };\n> \n> struct hunk_header {\n> -\tunsigned long old_offset, old_count, new_offset, new_count;\n> +\tsize_t old_offset, old_count, new_offset, new_count;\n> \t/*\n> \t * Start/end offsets to the extra text after the second `@@` in the\n> \t * hunk header, e.g. the function signature. This is expected to\n> @@ -322,11 +322,12 @@ static void setup_child_process(struct add_p_state *s,\n> }\n> \n> static int parse_range(const char **p,\n> -\t\t unsigned long *offset, unsigned long *count)\n> +\t\t size_t *offset, size_t *count)\n> {\n> \tchar *pend;\n> -\n> -\t*offset = strtoul(*p, &pend, 10);\n> +\t*offset = str_to_size_t(*p, &pend, 10);\n> +\tif (errno == ERANGE)\n> +\t\treturn error(_(\"Number is too large for this field\"));\n> \tif (pend == *p)\n> \t\treturn -1;\n> \tif (*pend != ',') {\n> @@ -334,7 +335,9 @@ static int parse_range(const char **p,\n> \t\t*p = pend;\n> \t\treturn 0;\n> \t}\n> -\t*count = strtoul(pend + 1, (char **)p, 10);\n> +\t*count = str_to_size_t(pend + 1, (char **)p, 10);\n> +\tif (errno == ERANGE)\n> +\t\treturn error(_(\"Number is too large for this field\"));\n> \treturn *p == pend + 1 ? -1 : 0;\n> }\n> \n> @@ -673,8 +676,8 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,\n> \t\t */\n> \t\tconst char *p;\n> \t\tsize_t len;\n> -\t\tunsigned long old_offset = header->old_offset;\n> -\t\tunsigned long new_offset = header->new_offset;\n> +\t\tsize_t old_offset = header->old_offset;\n> +\t\tsize_t new_offset = header->new_offset;\n> \n> \t\tif (!colored) {\n> \t\t\tp = s->plain.buf + header->extra_start;\n> @@ -700,12 +703,14 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,\n> \t\telse\n> \t\t\tnew_offset += delta;\n> \n> -\t\tstrbuf_addf(out, \"@@ -%lu\", old_offset);\n> +\t\tstrbuf_addf(out, \"@@ -%\" PRIuMAX, (uintmax_t)old_offset);\n> \t\tif (header->old_count != 1)\n> -\t\t\tstrbuf_addf(out, \",%lu\", header->old_count);\n> -\t\tstrbuf_addf(out, \" +%lu\", new_offset);\n> +\t\t\tstrbuf_addf(out, \",%\" PRIuMAX,\n> +\t\t\t\t (uintmax_t)header->old_count);\n> +\t\tstrbuf_addf(out, \" +%\" PRIuMAX, (uintmax_t)new_offset);\n> \t\tif (header->new_count != 1)\n> -\t\t\tstrbuf_addf(out, \",%lu\", header->new_count);\n> +\t\t\tstrbuf_addf(out, \",%\" PRIuMAX,\n> +\t\t\t\t (uintmax_t)header->new_count);\n> \t\tstrbuf_addstr(out, \" @@\");\n> \n> \t\tif (len)\n> @@ -1066,11 +1071,13 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,\n> \n> \t/* last hunk simply gets the rest */\n> \tif (header->old_offset != remaining.old_offset)\n> -\t\tBUG(\"miscounted old_offset: %lu != %lu\",\n> -\t\t header->old_offset, remaining.old_offset);\n> +\t\tBUG(\"miscounted old_offset: %\"PRIuMAX\" != %\"PRIuMAX,\n> +\t\t (uintmax_t)header->old_offset,\n> +\t\t (uintmax_t)remaining.old_offset);\n> \tif (header->new_offset != remaining.new_offset)\n> -\t\tBUG(\"miscounted new_offset: %lu != %lu\",\n> -\t\t header->new_offset, remaining.new_offset);\n> +\t\tBUG(\"miscounted new_offset: %\"PRIuMAX\" != %\"PRIuMAX,\n> +\t\t (uintmax_t)header->new_offset,\n> +\t\t (uintmax_t)remaining.new_offset);\n> \theader->old_count = remaining.old_count;\n> \theader->new_count = remaining.new_count;\n> \thunk->end = end;\n> @@ -1354,9 +1361,10 @@ static void summarize_hunk(struct add_p_state *s, struct hunk *hunk,\n> \tstruct strbuf *plain = &s->plain;\n> \tsize_t len = out->len, i;\n> \n> -\tstrbuf_addf(out, \" -%lu,%lu +%lu,%lu \",\n> -\t\t header->old_offset, header->old_count,\n> -\t\t header->new_offset, header->new_count);\n> +\tstrbuf_addf(out,\n> +\t\t \" -%\"PRIuMAX\",%\"PRIuMAX\" +%\"PRIuMAX\",%\"PRIuMAX\" \",\n> +\t\t (uintmax_t)header->old_offset, (uintmax_t)header->old_count,\n> +\t\t (uintmax_t)header->new_offset, (uintmax_t)header->new_count);\n> \tif (out->len - len < SUMMARY_HEADER_WIDTH)\n> \t\tstrbuf_addchars(out, ' ',\n> \t\t\t\tSUMMARY_HEADER_WIDTH + len - out->len);\n> @@ -1625,10 +1633,11 @@ static int patch_update_file(struct add_p_state *s,\n> \t\t\telse if (0 < response && response <= file_diff->hunk_nr)\n> \t\t\t\thunk_index = response - 1;\n> \t\t\telse\n> -\t\t\t\terr(s, Q_(\"Sorry, only %d hunk available.\",\n> -\t\t\t\t\t \"Sorry, only %d hunks available.\",\n> -\t\t\t\t\t file_diff->hunk_nr),\n> -\t\t\t\t (int)file_diff->hunk_nr);\n> +\t\t\t\terr(s,\n> +\t\t\t\t Q_(\"Sorry, only %\"PRIuMAX\" hunk available.\",\n> +\t\t\t\t \"Sorry, only %\"PRIuMAX\" hunks available.\",\n> +\t\t\t\t (uintmax_t)file_diff->hunk_nr),\n> +\t\t\t\t (uintmax_t)file_diff->hunk_nr);\n> \t\t} else if (s->answer.buf[0] == '/') {\n> \t\t\tregex_t regex;\n> \t\t\tint ret;\n> diff --git a/gettext.h b/gettext.h\n> index 484cafa562..d36f5a7ade 100644\n> --- a/gettext.h\n> +++ b/gettext.h\n> @@ -53,7 +53,7 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)\n> }\n> \n> static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)\n> -const char *Q_(const char *msgid, const char *plu, unsigned long n)\n> +const char *Q_(const char *msgid, const char *plu, size_t n)\n> {\n> \tif (!git_gettext_enabled)\n> \t\treturn n == 1 ? msgid : plu;\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index e283c46c6f..bb9a6c2bc4 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -292,6 +292,13 @@ static inline int _have_unix_sockets(void)\n> #include \n> #endif\n> \n> +#if SIZE_MAX == ULONG_MAX\n> +#define str_to_size_t strtoul\n> +#else\n> +#define str_to_size_t strtoull\n> +#endif\n> +\n> +\n> /* Used by compat/win32/path-utils.h, and more */\n> static inline int is_xplatform_dir_sep(int c)\n> {\n\n"},{"id":"511426","messageId":"xmqqsep1iei6.fsf@gitster.g","threadId":"62857","inReplyTo":"6a251603-25bc-415d-ab8c-ae698bd7977a@gmail.com","subject":"Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-29T19:52:49Z","receivedAt":"2025-01-29T19:52:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> I'm afraid I'm still not convinced this is a good idea for the reasons\n> I explained previously [1] together with an alternative approach to\n> silencing these warnings. What makes \"unsigned long\" an incorrect\n> choice when that's what \"git diff\" and \"git apply\" use?\n>\n> [1]\n> https://lore.kernel.org/git/e396131c-1bd3-46d0-bae6-cd97ca9710d8@gmail.com\n\nAh, this patch still does that? I was hoping that it got corrected\nalready after it was pointed out in the previous iterations. I\nagree with you that size_t is a dubious type to use for the line\nnumbers there.\n\ndiff.c defines \"struct emit_callback\" with lno_in_{pre,post}image\nmembers that are of type \"int\", which is somewhat dubious, too, but\nat least we don't run on 16-bit machines, and being limited to 2\nbillion lines is probably OK. I am OK to upgrade that to long (if\nwe use negative values for some oob signal) or ulong, but that is\nclearly outside of this topic.\n\n\n\n>> Add macro str_to_size_t for converting a string to size_t.\n>> Test if convertion fails with over or underflow.\n>\n> That is welcome, but the implementation needs tweaking. If you look at\n> other uses of strtoul() in our code you'll see that (somewhat\n> unusually) one needs to set errno to zero before calling strtoul() as\n> one cannot tell from the return value whether there was an error or\n> not. As errno may have been set by a previous function call it needs\n> to be cleared before calling strtoul() so we can be sure the error\n> came from strtoul().\n\nNice advice.\n\n> Best Wishes\n>\n> Phillip\n\nThanks.\n\n\nBy the way, who is\n\nand why is such an apparently bogus e-mail address Cc'ed?\n\n\n"},{"id":"511481","messageId":"57031bce-6dc4-48a7-b4b5-1b837ea3ab8f@gmail.com","threadId":"62857","inReplyTo":"xmqqsep1iei6.fsf@gitster.g","subject":"Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-01-30T10:47:22Z","receivedAt":"2025-01-30T10:47:27Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 29/01/2025 19:52, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n> By the way, who is\n> \n> and why is such an apparently bogus e-mail address Cc'ed?\n\nThat's the Reply-To address from the mail I was replying to. \nUnfortunately it does not seem to exist.\n\nBest Wishes\n\nPhillip\n\n\n"},{"id":"511521","messageId":"xmqqbjvocdgb.fsf@gitster.g","threadId":"62857","inReplyTo":"57031bce-6dc4-48a7-b4b5-1b837ea3ab8f@gmail.com","subject":"Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-30T19:24:20Z","receivedAt":"2025-01-30T19:24:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> On 29/01/2025 19:52, Junio C Hamano wrote:\n>> Phillip Wood writes:\n>> By the way, who is\n>> \n>> and why is such an apparently bogus e-mail address Cc'ed?\n>\n> That's the Reply-To address from the mail I was replying\n> to. Unfortunately it does not seem to exist.\n\nIt just occured to me that it is probably added by a mistake and the\nsender really wanted to add it to In-Reply-To: instead of Reply-To:\n\nI wonder if this is a mistake we can do something to help users\navoid? \"git send-email\" has the \"--reply-to=\" option and there is a\nvalid use case for that option, so disabling that option is a\nnon-starter.\n\nOf course there are other ways to send e-mailed patches, but I do\nnot think of a way to misuse them with reply-to and in-reply-to\nmixed up.\n\nThoughts?\n\n\n\n\n\n"}]}