{"thread":{"id":"62749","subject":"[PATCHv2 2/4] date.c: Fix type missmatch warings from msvc","startedAt":"2025-01-06T19:14:39Z","lastAt":"2025-01-07T00:53:28Z","messageCount":12,"participants":["Sören Krecker","Eric Sunshine","Andreas Schwab","brian m. carlson","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"510006","messageId":"20250106190855.3098-3-soekkle@freenet.de","threadId":"62749","inReplyTo":"20250106190855.3098-1-soekkle@freenet.de","subject":"[PATCHv2 2/4] date.c: Fix type missmatch warings from msvc","fromName":"Sören Krecker","fromEmail":"soekkle@freenet.de","sentAt":"2025-01-06T19:08:53Z","receivedAt":"2025-01-06T19:14:39Z","isPatch":false,"sender":{"key":"soekkle@freenet.de","avatar":"https://avatars.githubusercontent.com/u/6253399?v=4"},"body":"Fix compiler warings from msvc in date.c for value truncation from 64\nbit to 32 bit integers.\n\nAlso switch from int to size_t for all variables with result of strlen()\nwhich cannot become negative.\n\nSigned-off-by: Sören Krecker \n---\n date.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex a1b26a8dce..17a95077cf 100644\n--- a/date.c\n+++ b/date.c\n@@ -1244,7 +1244,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm\n \t}\n \n \tfor (s = special; s->name; s++) {\n-\t\tint len = strlen(s->name);\n+\t\tsize_t len = strlen(s->name);\n \t\tif (match_string(date, s->name) == len) {\n \t\t\ts->fn(tm, now, num);\n \t\t\t*touched = 1;\n@@ -1254,7 +1254,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm\n \n \tif (!*num) {\n \t\tfor (i = 1; i < 11; i++) {\n-\t\t\tint len = strlen(number_name[i]);\n+\t\t\tsize_t len = strlen(number_name[i]);\n \t\t\tif (match_string(date, number_name[i]) == len) {\n \t\t\t\t*num = i;\n \t\t\t\t*touched = 1;\n@@ -1270,7 +1270,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm\n \n \ttl = typelen;\n \twhile (tl->type) {\n-\t\tint len = strlen(tl->type);\n+\t\tsize_t len = strlen(tl->type);\n \t\tif (match_string(date, tl->type) >= len-1) {\n \t\t\tupdate_tm(tm, now, tl->length * *num);\n \t\t\t*num = 0;\n-- \n2.39.5\n\n"},{"id":"510007","messageId":"20250106190855.3098-1-soekkle@freenet.de","threadId":"62749","inReplyTo":null,"subject":"[PATCHv2 0/4] Fixes typemissmatch warinigs from msvc","fromName":"Sören Krecker","fromEmail":"soekkle@freenet.de","sentAt":"2025-01-06T19:08:51Z","receivedAt":"2025-01-06T19:14:41Z","isPatch":false,"sender":{"key":"soekkle@freenet.de","avatar":"https://avatars.githubusercontent.com/u/6253399?v=4"},"body":"Thanks for your replays. I try to improve this series and add under and\noverflow checks. To improve handling Platform specific stuff I add a macro\nfor conversion from a string to a size_t.\n\nBest regards\n\nSören Krecker\n\nSören Krecker (4):\n add-patch: Fix type missmatch rom msvc\n date.c: Fix type missmatch warings from msvc\n apply.c : Fix type missmatch warings from msvc\n commit.c: Fix type missmatch warings from msvc\n\n add-patch.c | 53 +++++++++++++++++++++++++++--------------------\n apply.c | 37 +++++++++++++++++----------------\n apply.h | 6 +++---\n commit.c | 12 +++++------\n date.c | 6 +++---\n gettext.h | 2 +-\n git-compat-util.h | 6 ++++++\n 7 files changed, 69 insertions(+), 53 deletions(-)\n\n\nbase-commit: 1b4e9a5f8b5f048972c21fe8acafe0404096f694\n-- \n2.39.5\n\n"},{"id":"510008","messageId":"20250106190855.3098-2-soekkle@freenet.de","threadId":"62749","inReplyTo":"20250106190855.3098-1-soekkle@freenet.de","subject":"[PATCHv2 1/4] add-patch: Fix type missmatch rom msvc","fromName":"Sören Krecker","fromEmail":"soekkle@freenet.de","sentAt":"2025-01-06T19:08:52Z","receivedAt":"2025-01-06T19:14:43Z","isPatch":false,"sender":{"key":"soekkle@freenet.de","avatar":"https://avatars.githubusercontent.com/u/6253399?v=4"},"body":"Fix some compiler warings from msvw 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 strtos for converting a string to size_t.\nTest if convertion fails with over or underflow.\n\nSigned-off-by: Sören Krecker \n\nUses strtouq\n\nimpove linux support\n\nChange Macro name\n---\n add-patch.c | 53 +++++++++++++++++++++++++++--------------------\n gettext.h | 2 +-\n git-compat-util.h | 6 ++++++\n 3 files changed, 38 insertions(+), 23 deletions(-)\n\ndiff --git a/add-patch.c b/add-patch.c\nindex 7b598e14df..67a7f68d23 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 = strtos(*p, &pend, 10);\n+\tif (errno == ERANGE)\n+\t\treturn error(\"Number dose not fit datatype\");\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 = strtos(pend + 1, (char **)p, 10);\n+\tif (errno == ERANGE)\n+\t\treturn error(\"Number dose not fit datatype\");\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..4c33990a05 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -291,6 +291,12 @@ static inline int _have_unix_sockets(void)\n #ifdef HAVE_BSD_SYSCTL\n #include \n #endif\n+#if defined _WIN64\n+# define strtos strtoull\n+#else\n+#define strtos strtoul\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-- \n2.39.5\n\n"},{"id":"510009","messageId":"20250106190855.3098-5-soekkle@freenet.de","threadId":"62749","inReplyTo":"20250106190855.3098-1-soekkle@freenet.de","subject":"[PATCHv2 4/4] commit.c: Fix type missmatch warings from msvc","fromName":"Sören Krecker","fromEmail":"soekkle@freenet.de","sentAt":"2025-01-06T19:08:55Z","receivedAt":"2025-01-06T19:14:46Z","isPatch":false,"sender":{"key":"soekkle@freenet.de","avatar":"https://avatars.githubusercontent.com/u/6253399?v=4"},"body":"Fix compiler warings from msvc in date.c for value truncation from 64\nbit to 32 bit integers.\n\nAlso switch from int to size_t for all variables with result of strlen()\nwhich cannot become negative.\n\nSigned-off-by: Sören Krecker \n---\n commit.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex a127fe60c5..78993395e6 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -466,8 +466,8 @@ int parse_commit_buffer(struct repository *r, struct commit *item, const void *b\n \tstruct object_id parent;\n \tstruct commit_list **pptr;\n \tstruct commit_graft *graft;\n-\tconst int tree_entry_len = the_hash_algo->hexsz + 5;\n-\tconst int parent_entry_len = the_hash_algo->hexsz + 7;\n+\tconst size_t tree_entry_len = the_hash_algo->hexsz + 5;\n+\tconst size_t parent_entry_len = the_hash_algo->hexsz + 7;\n \tstruct tree *tree;\n \n \tif (item->object.parsed)\n@@ -1114,10 +1114,10 @@ static const char *gpg_sig_headers[] = {\n \n int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct git_hash_algo *algo)\n {\n-\tint inspos, copypos;\n+\tssize_t inspos, copypos;\n \tconst char *eoh;\n \tconst char *gpg_sig_header = gpg_sig_headers[hash_algo_by_ptr(algo)];\n-\tint gpg_sig_header_len = strlen(gpg_sig_header);\n+\tsize_t gpg_sig_header_len = strlen(gpg_sig_header);\n \n \t/* find the end of the header */\n \teoh = strstr(buf->buf, \"\\n\\n\");\n@@ -1530,7 +1530,7 @@ int commit_tree(const char *msg, size_t msg_len, const struct object_id *tree,\n \treturn result;\n }\n \n-static int find_invalid_utf8(const char *buf, int len)\n+static int find_invalid_utf8(const char *buf, size_t len)\n {\n \tint offset = 0;\n \tstatic const unsigned int max_codepoint[] = {\n@@ -1539,7 +1539,7 @@ static int find_invalid_utf8(const char *buf, int len)\n \n \twhile (len) {\n \t\tunsigned char c = *buf++;\n-\t\tint bytes, bad_offset;\n+\t\tsize_t bytes, bad_offset;\n \t\tunsigned int codepoint;\n \t\tunsigned int min_val, max_val;\n \n-- \n2.39.5\n\n"},{"id":"510010","messageId":"20250106190855.3098-4-soekkle@freenet.de","threadId":"62749","inReplyTo":"20250106190855.3098-1-soekkle@freenet.de","subject":"[PATCHv2 3/4] apply.c : Fix type missmatch warings from msvc","fromName":"Sören Krecker","fromEmail":"soekkle@freenet.de","sentAt":"2025-01-06T19:08:54Z","receivedAt":"2025-01-06T19:14:50Z","isPatch":false,"sender":{"key":"soekkle@freenet.de","avatar":"https://avatars.githubusercontent.com/u/6253399?v=4"},"body":"Fix compiler warings from msvc in date.c for value truncation from 64\nbit to 32 bit integers.\n\nAlso switch from int to size_t for all variables with result of strlen()\nwhich cannot become negative.\n\nSigned-off-by: Sören Krecker \n---\n apply.c | 37 +++++++++++++++++++------------------\n apply.h | 6 +++---\n 2 files changed, 22 insertions(+), 21 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 4a7b6120ac..b896889505 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -414,9 +414,9 @@ static int read_patch_file(struct strbuf *sb, int fd)\n \treturn 0;\n }\n \n-static unsigned long linelen(const char *buffer, unsigned long size)\n+static size_t linelen(const char *buffer, size_t size)\n {\n-\tunsigned long len = 0;\n+\tsize_t len = 0;\n \twhile (size--) {\n \t\tlen++;\n \t\tif (*buffer++ == '\\n')\n@@ -688,7 +688,7 @@ static char *find_name_common(struct strbuf *root,\n \t * or \"file~\").\n \t */\n \tif (def) {\n-\t\tint deflen = strlen(def);\n+\t\tsize_t deflen = strlen(def);\n \t\tif (deflen < len && !strncmp(start, def, deflen))\n \t\t\treturn squash_slash(xstrdup(def));\n \t}\n@@ -1088,7 +1088,7 @@ static int gitdiff_index(struct gitdiff_data *state,\n \t */\n \tconst char *ptr, *eol;\n \tint len;\n-\tconst unsigned hexsz = the_hash_algo->hexsz;\n+\tconst size_t hexsz = the_hash_algo->hexsz;\n \n \tptr = strchr(line, '.');\n \tif (!ptr || ptr[1] != '.' || hexsz < ptr - line)\n@@ -1131,7 +1131,7 @@ static int gitdiff_unrecognized(struct gitdiff_data *state UNUSED,\n */\n static const char *skip_tree_prefix(int p_value,\n \t\t\t\t const char *line,\n-\t\t\t\t int llen)\n+\t\t\t\t size_t llen)\n {\n \tint nslash;\n \tint i;\n@@ -1158,7 +1158,7 @@ static const char *skip_tree_prefix(int p_value,\n */\n static char *git_header_name(int p_value,\n \t\t\t const char *line,\n-\t\t\t int llen)\n+\t\t\t ssize_t llen)\n {\n \tconst char *name;\n \tconst char *second = NULL;\n@@ -1313,15 +1313,15 @@ static int check_header_line(int linenr, struct patch *patch)\n \treturn 0;\n }\n \n-int parse_git_diff_header(struct strbuf *root,\n+size_t parse_git_diff_header(struct strbuf *root,\n \t\t\t int *linenr,\n \t\t\t int p_value,\n \t\t\t const char *line,\n-\t\t\t int len,\n-\t\t\t unsigned int size,\n+\t\t\t size_t len,\n+\t\t\t size_t size,\n \t\t\t struct patch *patch)\n {\n-\tunsigned long offset;\n+\tsize_t offset;\n \tstruct gitdiff_data parse_hdr_state;\n \n \t/* A git diff has explicit new/delete information, so we don't guess */\n@@ -1378,7 +1378,7 @@ int parse_git_diff_header(struct strbuf *root,\n \t\t\tbreak;\n \t\tfor (i = 0; i < ARRAY_SIZE(optable); i++) {\n \t\t\tconst struct opentry *p = optable + i;\n-\t\t\tint oplen = strlen(p->str);\n+\t\t\tsize_t oplen = strlen(p->str);\n \t\t\tint res;\n \t\t\tif (len < oplen || memcmp(p->str, line, oplen))\n \t\t\t\tcontinue;\n@@ -1430,7 +1430,8 @@ static int parse_num(const char *line, unsigned long *p)\n static int parse_range(const char *line, int len, int offset, const char *expect,\n \t\t unsigned long *p1, unsigned long *p2)\n {\n-\tint digits, ex;\n+\tint digits;\n+\tsize_t ex;\n \n \tif (offset < 0 || offset >= len)\n \t\treturn -1;\n@@ -1465,7 +1466,7 @@ static int parse_range(const char *line, int len, int offset, const char *expect\n \treturn offset + ex;\n }\n \n-static void recount_diff(const char *line, int size, struct fragment *fragment)\n+static void recount_diff(const char *line, size_t size, struct fragment *fragment)\n {\n \tint oldlines = 0, newlines = 0, ret = 0;\n \n@@ -1475,7 +1476,7 @@ static void recount_diff(const char *line, int size, struct fragment *fragment)\n \t}\n \n \tfor (;;) {\n-\t\tint len = linelen(line, size);\n+\t\tsize_t len = linelen(line, size);\n \t\tsize -= len;\n \t\tline += len;\n \n@@ -1543,11 +1544,11 @@ static int parse_fragment_header(const char *line, int len, struct fragment *fra\n */\n static int find_header(struct apply_state *state,\n \t\t const char *line,\n-\t\t unsigned long size,\n+\t\t size_t size,\n \t\t int *hdrsize,\n \t\t struct patch *patch)\n {\n-\tunsigned long offset, len;\n+\tsize_t offset, len;\n \n \tpatch->is_toplevel_relative = 0;\n \tpatch->is_rename = patch->is_copy = 0;\n@@ -2132,7 +2133,7 @@ static int use_patch(struct apply_state *state, struct patch *p)\n * the number of bytes consumed otherwise,\n * so that the caller can call us again for the next patch.\n */\n-static int parse_chunk(struct apply_state *state, char *buffer, unsigned long size, struct patch *patch)\n+static int parse_chunk(struct apply_state *state, char *buffer, size_t size, struct patch *patch)\n {\n \tint hdrsize, patchsize;\n \tint offset = find_header(state, buffer, size, &hdrsize, patch);\n@@ -2491,7 +2492,7 @@ static int match_fragment(struct apply_state *state,\n \tstruct strbuf fixed = STRBUF_INIT;\n \tchar *fixed_buf;\n \tsize_t fixed_len;\n-\tint preimage_limit;\n+\tssize_t preimage_limit;\n \tint ret;\n \n \tif (preimage->line_nr + current_lno <= img->line_nr) {\ndiff --git a/apply.h b/apply.h\nindex 90e887ec0e..bb01ce7dbc 100644\n--- a/apply.h\n+++ b/apply.h\n@@ -166,12 +166,12 @@ int check_apply_state(struct apply_state *state, int force_apply);\n *\n * Returns -1 on failure, the length of the parsed header otherwise.\n */\n-int parse_git_diff_header(struct strbuf *root,\n+size_t parse_git_diff_header(struct strbuf *root,\n \t\t\t int *linenr,\n \t\t\t int p_value,\n \t\t\t const char *line,\n-\t\t\t int len,\n-\t\t\t unsigned int size,\n+\t\t\t size_t len,\n+\t\t\t size_t size,\n \t\t\t struct patch *patch);\n \n void release_patch(struct patch *patch);\n-- \n2.39.5\n\n"},{"id":"510022","messageId":"CAPig+cR0GgZQ+XCyAh=xHRfjfsAdTzC2uyML1vnoM_fXv1Bxew@mail.gmail.com","threadId":"62749","inReplyTo":"20250106190855.3098-3-soekkle@freenet.de","subject":"Re: [PATCHv2 2/4] date.c: Fix type missmatch warings from msvc","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-01-06T22:22:06Z","receivedAt":"2025-01-06T22:22:18Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jan 6, 2025 at 2:14 PM Sören Krecker wrote:\n> Fix compiler warings from msvc in date.c for value truncation from 64\n> bit to 32 bit integers.\n\ns/warings/warnings/\n\n> Also switch from int to size_t for all variables with result of strlen()\n> which cannot become negative.\n>\n> Signed-off-by: Sören Krecker \n> ---\n> diff --git a/date.c b/date.c\n> @@ -1270,7 +1270,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm\n> tl = typelen;\n> while (tl->type) {\n> - int len = strlen(tl->type);\n> + size_t len = strlen(tl->type);\n> if (match_string(date, tl->type) >= len-1) {\n\nThis change looks scary and potentially wrong considering that the\nexpression in the `if` statement subtracts 1 from `len`. If `len`\nhappens to be zero, then `len-1` will wrap around to a very large\nnumber, thus potentially changing the meaning of the `if` condition.\n\nNow, admittedly, I haven't delved into this code or thought about it\nmuch, so I may be entirely wrong about this; perhaps it is impossible\nfor `len` to ever be zero in this context or perhaps the meaning of\nthe `if` condition doesn't change even if it wraps around. But if\nthat's the case, you should use the commit message to explain to\nreaders that you have audited the code and verified that `len` will\nnever be zero or that the condition remains safe despite wraparound.\nAlso, even if you verify that this change is perfectly safe, because\nit _appears_ to be a potentially behavior breaking change, you should\nisolate it in its own commit, separate from the other changes, to let\nreviewers know that it deserves special scrutiny.\n"},{"id":"510024","messageId":"CAPig+cQ0cmK_ttjt8tKWEzYpF1-gJMzFS_N0imXH+7rVezk-XQ@mail.gmail.com","threadId":"62749","inReplyTo":"20250106190855.3098-4-soekkle@freenet.de","subject":"Re: [PATCHv2 3/4] apply.c : Fix type missmatch warings from msvc","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-01-06T22:26:42Z","receivedAt":"2025-01-06T22:26:54Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jan 6, 2025 at 2:15 PM Sören Krecker wrote:\n> Fix compiler warings from msvc in date.c for value truncation from 64\n> bit to 32 bit integers.\n\ns/warings/warnings/\ns/date.c/apply.c/\n\n> Also switch from int to size_t for all variables with result of strlen()\n> which cannot become negative.\n>\n> Signed-off-by: Sören Krecker \n"},{"id":"510025","messageId":"CAPig+cQ49Hdc_8=mRhhJDTny_Kqo6Wg6Nr98rsBN_YXmBrQ6kA@mail.gmail.com","threadId":"62749","inReplyTo":"20250106190855.3098-5-soekkle@freenet.de","subject":"Re: [PATCHv2 4/4] commit.c: Fix type missmatch warings from msvc","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-01-06T22:27:28Z","receivedAt":"2025-01-06T22:27:40Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jan 6, 2025 at 2:15 PM Sören Krecker wrote:\n> Fix compiler warings from msvc in date.c for value truncation from 64\n> bit to 32 bit integers.\n\ns/warings/warnings/\ns/date.c/commit.c/\n\n> Also switch from int to size_t for all variables with result of strlen()\n> which cannot become negative.\n>\n> Signed-off-by: Sören Krecker \n"},{"id":"510029","messageId":"87sepvpna6.fsf@igel.home","threadId":"62749","inReplyTo":"CAPig+cR0GgZQ+XCyAh=xHRfjfsAdTzC2uyML1vnoM_fXv1Bxew@mail.gmail.com","subject":"Re: [PATCHv2 2/4] date.c: Fix type missmatch warings from msvc","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2025-01-06T22:53:21Z","receivedAt":"2025-01-06T23:03:24Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Jan 06 2025, Eric Sunshine wrote:\n\n> On Mon, Jan 6, 2025 at 2:14 PM Sören Krecker wrote:\n>> Fix compiler warings from msvc in date.c for value truncation from 64\n>> bit to 32 bit integers.\n>\n> s/warings/warnings/\n>\n>> Also switch from int to size_t for all variables with result of strlen()\n>> which cannot become negative.\n>>\n>> Signed-off-by: Sören Krecker \n>> ---\n>> diff --git a/date.c b/date.c\n>> @@ -1270,7 +1270,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm\n>> tl = typelen;\n>> while (tl->type) {\n>> - int len = strlen(tl->type);\n>> + size_t len = strlen(tl->type);\n>> if (match_string(date, tl->type) >= len-1) {\n>\n> This change looks scary and potentially wrong considering that the\n> expression in the `if` statement subtracts 1 from `len`. If `len`\n> happens to be zero, then `len-1` will wrap around to a very large\n> number, thus potentially changing the meaning of the `if` condition.\n>\n> Now, admittedly, I haven't delved into this code or thought about it\n> much, so I may be entirely wrong about this; perhaps it is impossible\n> for `len` to ever be zero in this context or perhaps the meaning of\n> the `if` condition doesn't change even if it wraps around.\n\nIt can be made more robust by moving the constant to the other side:\n\n if (match_string(date, tl->type)+1 >= len) {\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1\n\"And now for something completely different.\"\n"},{"id":"510033","messageId":"Z3xxxbKtqyLmDAif@tapette.crustytoothpaste.net","threadId":"62749","inReplyTo":"20250106190855.3098-2-soekkle@freenet.de","subject":"Re: [PATCHv2 1/4] add-patch: Fix type missmatch rom msvc","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-01-07T00:13:57Z","receivedAt":"2025-01-07T00:14:00Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-01-06 at 19:08:52, Sören Krecker wrote:\n> Fix some compiler warings from msvw 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 strtos for converting a string to size_t.\n> Test if convertion fails with over or underflow.\n\nA few minor nits here. We want to say \"from\" both here and in the title\nand \"conversion\" (and in the title, \"mismatch\"), and put a space after\nthe period in a sentence. I think you meant \"MSVC\" instead of \"msvw\",\nbut if not, please do explain what that is, since I'm not familiar with\nit and I'm curious. The commit message is a good place to explain lots\nin detail.\n\n> Signed-off-by: Sören Krecker \n> \n> Uses strtouq\n\nI don't see that we're using this function.\n\n> impove linux support\n> \n> Change Macro name\n\nWe don't typically put comments about the revisions we've made to a\npatch in the commit message. We may put them below the --- so that\nthey're visible to readers and reviewers, which is helpful, but we\npretend that our patches were perfect to begin with in terms of the\ncommit message, since the future reader of the history only cares about\nthe actual end result and not what changes we made along the way.\n\n> ---\n> add-patch.c | 53 +++++++++++++++++++++++++++--------------------\n> gettext.h | 2 +-\n> git-compat-util.h | 6 ++++++\n> 3 files changed, 38 insertions(+), 23 deletions(-)\n> \n> diff --git a/add-patch.c b/add-patch.c\n> index 7b598e14df..67a7f68d23 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 = strtos(*p, &pend, 10);\n\nI see you've defined this below.\n\n> +\tif (errno == ERANGE)\n> +\t\treturn error(\"Number dose not fit datatype\");\n\nI think the word you wanted was \"does\". However, perhaps we should\nprovide a better, more meaningful error message so the user knows what\ndata they provided that was invalid. Maybe \"absurdly large value in\ndiff header range\"? It would be quite bizarre to get a value even as\nlarge as the maximum value of a 32-bit integer, and I don't think our\ndiff code can even handle values larger than INT_MAX.\n\nIn that context, it might not even be necessary to handle values larger\nthan unsigned long, since we can't generate them. However, in the\ninterests of compatibility with other implementations which might not\nhave that limitation, size_t seems reasonable as a choice to handle more\ngenerally.\n\nAssuming we keep this, we probably also want to mark this for\ntranslation by wrapping it in `_(` and `)`.\n\nI also don't think this order is correct. In general, errno is not\nreset implicitly, so unless we know that an error occurred, errno is\nmeaningless, since another function could have set it to ERANGE. We'd\nprobably need to save errno, set it to 0, and restore to verify that we\ngot the right value, since we can't distinguish here between a truncated\nvalue for range reasons and for other reasons.\n\n> \tif (pend == *p)\n> \t\treturn -1;\n>\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 = strtos(pend + 1, (char **)p, 10);\n> +\tif (errno == ERANGE)\n> +\t\treturn error(\"Number dose not fit datatype\");\n\nSame comment here.\n\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\nIf we're using size_t, we can use %zu. That's specified in C99 as the\nappropriate formatting type for size_t, and we require C99 or C11 for\nall systems. We don't need to cast to uintmax_t.\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> diff --git a/git-compat-util.h b/git-compat-util.h\n> index e283c46c6f..4c33990a05 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -291,6 +291,12 @@ static inline int _have_unix_sockets(void)\n> #ifdef HAVE_BSD_SYSCTL\n> #include \n> #endif\n> +#if defined _WIN64\n> +# define strtos strtoull\n> +#else\n> +#define strtos strtoul\n> +#endif\n\nThis is not a great name for the function. First of all, it resembles\nthe standard functions a lot, so it's something that POSIX could\nstandardize or an OS could add, and then we'll have some fun compilation\nerrors when we redefine things.\n\nSecond, it's a lot less future-proof. While I do agree that only\nWindows 64-bit systems are likely to fall into this case, since we\nalready include , we probably should do this:\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(or whatever you want to call the function).\n\nThat expresses what we care about—that the type is suitable for the\nvalue we want to parse—and doesn't use the OS as a proxy for that data.\nOtherwise, the Unix developer who doesn't use Windows may not\nunderstand _why_ Windows is special and the reason we've chosen this\nchange.\n\nOn that note, it would be helpful if you explained in the commit message\nwhy that is for people who don't know. Maybe something like this:\n\n On 64-bit systems, size_t is a 64-bit type. On most Unix systems,\n unsigned long is also 64 bits in size, so we can use functions for\n that type to parse values of size_t. However, on Windows, unsigned\n long is always 32 bits, and if we want a 64-bit type, we must use\n unsigned long long. To future-proof our changes against other\n platforms that might be added in the future, we first check if\n unsigned long is sufficient, and otherwise, use unsigned long long,\n which will work in both cases.\n\nOf course, please feel free to edit as you see fit.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"510034","messageId":"xmqqed1fwjt9.fsf@gitster.g","threadId":"62749","inReplyTo":"Z3xxxbKtqyLmDAif@tapette.crustytoothpaste.net","subject":"Re: [PATCHv2 1/4] add-patch: Fix type missmatch rom msvc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-07T00:26:26Z","receivedAt":"2025-01-07T00:26:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> We don't typically put comments about the revisions we've made to a\n> patch in the commit message. We may put them below the --- so that\n> they're visible to readers and reviewers, which is helpful, but we\n> pretend that our patches were perfect to begin with in terms of the\n> commit message, since the future reader of the history only cares about\n> the actual end result and not what changes we made along the way.\n\nThanks, all true. The future readers would only *see* the end\nresults, and we do not want to hear about the previous stumblings\nthe author made before reaching an acceptable version.\n\n>> ---\n>> add-patch.c | 53 +++++++++++++++++++++++++++--------------------\n>> gettext.h | 2 +-\n>> git-compat-util.h | 6 ++++++\n>> 3 files changed, 38 insertions(+), 23 deletions(-)\n\nI already made this comment, but I think the offset/count being\nulong is a very sane design decision, and what is causing the\ncompiler warning is some earlier change that introduced size_t\nvariables or parameters in the callchain. As far as I can tell,\nthere is no system functions that yields size_t (hence we must use\nsize_t everywhere) in the code paths that deal with offset and\ncount. I suggested to find these abused size_t and fix them to use\nthe matching type, i.e. \"unsigned long\", instead, as an alternative\nfix. I did not get an impression that the author tried the approach\nand found why we must use size_t for offset/count instead.\n\nAnd if we go that route, there is *no* need to talk about 64-bit ve\n32-bit platforms. ulong used consistently everywhere would let you\nuse offset/count that fits in ulong, and the apply machinery is\nartificially limited to limit the patch size to a few gigabytes, so\n32-bit ulong should be plenty as Phillip pointed out earlier.\n\nIf we needed to parse an integer into a large integer, the existing\ncode seem to use strtoumax() into uintmax_t and move it to the\ntarget (while checking for truncation). \"Ah we are on windows, so\nuse strtoll, otherwise use strtol\" is not something we want to see\nin our codebase.\n\n> If we're using size_t, we can use %zu. That's specified in C99 as the\n> appropriate formatting type for size_t, and we require C99 or C11 for\n> all systems. We don't need to cast to uintmax_t.\n\nYou and Documentation/CodingGuidelines contradict with each other\nhere.\n\nThanks for a review.\n"},{"id":"510035","messageId":"xmqqa5c3wikb.fsf@gitster.g","threadId":"62749","inReplyTo":"xmqqed1fwjt9.fsf@gitster.g","subject":"Re: [PATCHv2 1/4] add-patch: Fix type missmatch rom msvc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-07T00:53:24Z","receivedAt":"2025-01-07T00:53:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> If we're using size_t, we can use %zu. That's specified in C99 as the\n>> appropriate formatting type for size_t, and we require C99 or C11 for\n>> all systems. We don't need to cast to uintmax_t.\n>\n> You and Documentation/CodingGuidelines contradict with each other\n> here.\n\nBy this, I do not necessarily mean that we should stick to the past\ntradition since d7d850e2 (CodingGuidelines: mention C99 features we\ncan't use, 2022-10-10), written back when MSVC was claiming to do\nC99 without letting us use %z conversion.\n\nWhat I meant was that if we are to update our stance against %z\nconversion after re-evaluating the situation (and such time will\ncertainly come someday---I do not offhand know if it can be today),\nwe should update the documentation before or at least at the same\ntime we recommend its use to new people.\n\nThanks.\n\n"}]}