{"thread":{"id":"57514","subject":"[PATCH 1/2] gettext API users: correct use of casts for Q_()","startedAt":"2022-03-07T11:38:21Z","lastAt":"2022-03-07T23:34:40Z","messageCount":15,"participants":["Ævar Arnfjörð Bjarmason","Derrick Stolee","Philip Oakley","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"450530","messageId":"patch-1.2-83659fbc459-20220307T113707Z-avarab@gmail.com","threadId":"57514","inReplyTo":"cover-0.2-00000000000-20220307T113707Z-avarab@gmail.com","subject":"[PATCH 1/2] gettext API users: correct use of casts for Q_()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T11:38:00Z","receivedAt":"2022-03-07T11:38:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change users of the inline gettext.h Q_() function to cast its\nargument to \"unsigned long\" instead of \"int\" or \"unsigned int\".\n\nThe ngettext() function (which Q_() resolves to) takes an \"unsigned\nlong int\", and so does our Q_() wrapper for it, see 0c9ea33b90f (i18n:\nadd stub Q_() wrapper for ngettext, 2011-03-09).\n\nIn a subsequent commit we'll be making more use of this pattern of:\n\n    func(Q_(..%\"PRIuMAX..., (unsigned long)x), (uintmax_t)x);\n\nBy making this change we ensure that this case isn't the odd one out\nin that post-image.\n\nThis:\n\n * Corrects code added in 7171a0b0cf5 (index-pack: correct \"len\" type\n   in unpack_data(), 2016-07-13) to cast the \"off_t len\" to an\n   \"unsigned long int\" rather than an \"unsigned int\".\n\n * Does the same for code in add-interactive.c added in several\n   commits starting with a8c45be939d (built-in add -i: implement the\n   `update` command, 2019-11-29).\n\n * Likewise for a case in 9254bdfb4f9 (built-in add -p: implement the\n   'g' (\"goto\") command, 2019-12-13) where only the err() argument had\n   a cast, but not the same argument to Q_().\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n add-interactive.c    | 15 +++++++++------\n add-patch.c          |  8 ++++----\n builtin/index-pack.c |  2 +-\n 3 files changed, 14 insertions(+), 11 deletions(-)\n\ndiff --git a/add-interactive.c b/add-interactive.c\nindex e1ab39cce30..6da781004ad 100644\n--- a/add-interactive.c\n+++ b/add-interactive.c\n@@ -707,8 +707,9 @@ static int run_update(struct add_i_state *s, const struct pathspec *ps,\n \t\tres = error(_(\"could not write index\"));\n \n \tif (!res)\n-\t\tprintf(Q_(\"updated %d path\\n\",\n-\t\t\t  \"updated %d paths\\n\", count), (int)count);\n+\t\tprintf(Q_(\"updated %\"PRIuMAX\" path\\n\",\n+\t\t\t  \"updated %\"PRIuMAX\" paths\\n\", (unsigned long)count),\n+\t\t       (uintmax_t)count);\n \n \tputchar('\\n');\n \treturn res;\n@@ -814,8 +815,9 @@ static int run_revert(struct add_i_state *s, const struct pathspec *ps,\n \t\t\t\t\t\t   NULL, NULL, NULL);\n \n \tif (!res)\n-\t\tprintf(Q_(\"reverted %d path\\n\",\n-\t\t\t  \"reverted %d paths\\n\", count), (int)count);\n+\t\tprintf(Q_(\"reverted %\"PRIuMAX\" path\\n\",\n+\t\t\t  \"reverted %\"PRIuMAX\" paths\\n\", (unsigned long)count),\n+\t\t       (uintmax_t)count);\n \n finish_revert:\n \tputchar('\\n');\n@@ -896,8 +898,9 @@ static int run_add_untracked(struct add_i_state *s, const struct pathspec *ps,\n \t\tres = error(_(\"could not write index\"));\n \n \tif (!res)\n-\t\tprintf(Q_(\"added %d path\\n\",\n-\t\t\t  \"added %d paths\\n\", count), (int)count);\n+\t\tprintf(Q_(\"added %\"PRIuMAX\" path\\n\",\n+\t\t\t  \"added %\"PRIuMAX\" paths\\n\", (unsigned long)count),\n+\t\t       (uintmax_t)count);\n \n finish_add_untracked:\n \tputchar('\\n');\ndiff --git a/add-patch.c b/add-patch.c\nindex 55d719f7845..dfef00e5680 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -1569,10 +1569,10 @@ 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, Q_(\"Sorry, only %\"PRIuMAX\" hunk available.\",\n+\t\t\t\t\t  \"Sorry, only %\"PRIuMAX\" hunks available.\",\n+\t\t\t\t\t  (unsigned long)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/builtin/index-pack.c b/builtin/index-pack.c\nindex 3c2e6aee3cc..f15b59e22b0 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -579,7 +579,7 @@ static void *unpack_data(struct object_entry *obj,\n \t\tif (!n)\n \t\t\tdie(Q_(\"premature end of pack file, %\"PRIuMAX\" byte missing\",\n \t\t\t       \"premature end of pack file, %\"PRIuMAX\" bytes missing\",\n-\t\t\t       (unsigned int)len),\n+\t\t\t       (unsigned long)len),\n \t\t\t    (uintmax_t)len);\n \t\tfrom += n;\n \t\tlen -= n;\n-- \n2.35.1.1241.gd8d69a17521\n\n"},{"id":"450531","messageId":"cover-0.2-00000000000-20220307T113707Z-avarab@gmail.com","threadId":"57514","inReplyTo":null,"subject":"[PATCH 0/2] string-list.h: make \"nr\" and \"alloc\" a \"size_t\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T11:37:59Z","receivedAt":"2022-03-07T11:38:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"$subject most recently came up in\nhttps://lore.kernel.org/git/220305.86pmn030ou.gmgdl@evledraar.gmail.com/. I.e. we\nhave code in-tree already that's possibly (although in that case\nunlikely to in practice) keeping track of size_t-indexed with a\nstring_list's \"unsigned int\"-indexed data.\n\nNow there are no textual or semantic conflicts between this change and\n\"seen\", so having series (which is of course based on \"master\") now\nwould be a good thing, and help to future-proof the API.\n\nThe 1/2 here isn't strictly neccesary as part of this series, but\nsince a large part 2/2 is adjusting the same sort of Q_() patterns I\nthought it made sense to both show existing PRIuMAX use in those\nmacros, and how we do those casts.\n\nÆvar Arnfjörð Bjarmason (2):\n  gettext API users: correct use of casts for Q_()\n  string-list API: change \"nr\" and \"alloc\" to \"size_t\"\n\n add-interactive.c           | 15 +++++++++------\n add-patch.c                 |  8 ++++----\n builtin/index-pack.c        |  2 +-\n builtin/receive-pack.c      |  9 +++++----\n builtin/shortlog.c          | 10 +++++-----\n bundle.c                    | 12 ++++++------\n commit-graph.c              |  8 ++++----\n mailmap.c                   |  7 ++++---\n merge-ort.c                 |  4 ++--\n string-list.h               |  3 ++-\n t/helper/test-run-command.c |  7 ++++---\n wt-status.c                 | 16 ++++++++--------\n 12 files changed, 54 insertions(+), 47 deletions(-)\n\n-- \n2.35.1.1241.gd8d69a17521\n\n"},{"id":"450532","messageId":"patch-2.2-398682c07aa-20220307T113707Z-avarab@gmail.com","threadId":"57514","inReplyTo":"cover-0.2-00000000000-20220307T113707Z-avarab@gmail.com","subject":"[PATCH 2/2] string-list API: change \"nr\" and \"alloc\" to \"size_t\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T11:38:01Z","receivedAt":"2022-03-07T11:38:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"nr\" and \"alloc\" members of \"struct string_list\" to use\n\"size_t\" instead of \"nr\". On some platforms the size of an \"unsigned\nint\" will be smaller than a \"size_t\", e.g. a 32 bit unsigned v.s. 64\nbit unsigned. As \"struct string_list\" is a generic API we use in a lot\nof places this might cause overflows.\n\nAs one example: code in \"refs.c\" keeps track of the number of refs\nwith a \"size_t\", and auxiliary code in builtin/remote.c in\nget_ref_states() appends those to a \"struct string_list\".\n\nWhile we're at it split the \"nr\" and \"alloc\" in string-list.h across\ntwo lines, which is the case for most such struct member\ndeclarations (e.g. in \"strbuf.h\" and \"strvec.h\").\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/receive-pack.c      |  9 +++++----\n builtin/shortlog.c          | 10 +++++-----\n bundle.c                    | 12 ++++++------\n commit-graph.c              |  8 ++++----\n mailmap.c                   |  7 ++++---\n merge-ort.c                 |  4 ++--\n string-list.h               |  3 ++-\n t/helper/test-run-command.c |  7 ++++---\n wt-status.c                 | 16 ++++++++--------\n 9 files changed, 40 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex d10aeb7e78f..fc948a27c4f 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -813,13 +813,14 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed,\n \tproc.trace2_hook_name = hook_name;\n \n \tif (feed_state->push_options) {\n-\t\tint i;\n+\t\tsize_t i;\n \t\tfor (i = 0; i < feed_state->push_options->nr; i++)\n \t\t\tstrvec_pushf(&proc.env_array,\n-\t\t\t\t     \"GIT_PUSH_OPTION_%d=%s\", i,\n+\t\t\t\t     \"GIT_PUSH_OPTION_%\"PRIuMAX\"=%s\",\n+\t\t\t\t     (uintmax_t)i,\n \t\t\t\t     feed_state->push_options->items[i].string);\n-\t\tstrvec_pushf(&proc.env_array, \"GIT_PUSH_OPTION_COUNT=%d\",\n-\t\t\t     feed_state->push_options->nr);\n+\t\tstrvec_pushf(&proc.env_array, \"GIT_PUSH_OPTION_COUNT=%\"PRIuMAX\"\",\n+\t\t\t     (uintmax_t)feed_state->push_options->nr);\n \t} else\n \t\tstrvec_pushf(&proc.env_array, \"GIT_PUSH_OPTION_COUNT\");\n \ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 228d782754a..26c5c0cf935 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -435,7 +435,7 @@ static void add_wrapped_shortlog_msg(struct strbuf *sb, const char *s,\n \n void shortlog_output(struct shortlog *log)\n {\n-\tint i, j;\n+\tsize_t i, j;\n \tstruct strbuf sb = STRBUF_INIT;\n \n \tif (log->sort_by_number)\n@@ -448,10 +448,10 @@ void shortlog_output(struct shortlog *log)\n \t\t\t\t(int)UTIL_TO_INT(item), item->string);\n \t\t} else {\n \t\t\tstruct string_list *onelines = item->util;\n-\t\t\tfprintf(log->file, \"%s (%d):\\n\",\n-\t\t\t\titem->string, onelines->nr);\n-\t\t\tfor (j = onelines->nr - 1; j >= 0; j--) {\n-\t\t\t\tconst char *msg = onelines->items[j].string;\n+\t\t\tfprintf(log->file, \"%s (%\"PRIuMAX\"):\\n\",\n+\t\t\t\titem->string, (uintmax_t)onelines->nr);\n+\t\t\tfor (j = onelines->nr; j >= 1; j--) {\n+\t\t\t\tconst char *msg = onelines->items[j - 1].string;\n \n \t\t\t\tif (log->wrap_lines) {\n \t\t\t\t\tstrbuf_reset(&sb);\ndiff --git a/bundle.c b/bundle.c\nindex a0bb687b0f4..0ebc8733679 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -255,18 +255,18 @@ int verify_bundle(struct repository *r,\n \n \t\tr = &header->references;\n \t\tprintf_ln(Q_(\"The bundle contains this ref:\",\n-\t\t\t     \"The bundle contains these %d refs:\",\n-\t\t\t     r->nr),\n-\t\t\t  r->nr);\n+\t\t\t     \"The bundle contains these %\"PRIuMAX\" refs:\",\n+\t\t\t     (unsigned long)r->nr),\n+\t\t\t  (uintmax_t)r->nr);\n \t\tlist_refs(r, 0, NULL);\n \t\tr = &header->prerequisites;\n \t\tif (!r->nr) {\n \t\t\tprintf_ln(_(\"The bundle records a complete history.\"));\n \t\t} else {\n \t\t\tprintf_ln(Q_(\"The bundle requires this ref:\",\n-\t\t\t\t     \"The bundle requires these %d refs:\",\n-\t\t\t\t     r->nr),\n-\t\t\t\t  r->nr);\n+\t\t\t\t     \"The bundle requires these %\"PRIuMAX\" refs:\",\n+\t\t\t\t     (unsigned long)r->nr),\n+\t\t\t\t  (uintmax_t)r->nr);\n \t\t\tlist_refs(r, 0, NULL);\n \t\t}\n \t}\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 265c010122e..c7584f802b0 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1690,10 +1690,10 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,\n \tdirlen = packname.len;\n \tif (ctx->report_progress) {\n \t\tstrbuf_addf(&progress_title,\n-\t\t\t    Q_(\"Finding commits for commit graph in %d pack\",\n-\t\t\t       \"Finding commits for commit graph in %d packs\",\n-\t\t\t       pack_indexes->nr),\n-\t\t\t    pack_indexes->nr);\n+\t\t\t    Q_(\"Finding commits for commit graph in %\"PRIuMAX\" pack\",\n+\t\t\t       \"Finding commits for commit graph in %\"PRIuMAX\" packs\",\n+\t\t\t       (unsigned long)pack_indexes->nr),\n+\t\t\t    (uintmax_t)pack_indexes->nr);\n \t\tctx->progress = start_delayed_progress(progress_title.buf, 0);\n \t\tctx->progress_done = 0;\n \t}\ndiff --git a/mailmap.c b/mailmap.c\nindex 40ce152024d..7befdc5e483 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -43,8 +43,8 @@ static void free_mailmap_info(void *p, const char *s)\n static void free_mailmap_entry(void *p, const char *s)\n {\n \tstruct mailmap_entry *me = (struct mailmap_entry *)p;\n-\tdebug_mm(\"mailmap: removing entries for <%s>, with %d sub-entries\\n\",\n-\t\t s, me->namemap.nr);\n+\tdebug_mm(\"mailmap: removing entries for <%s>, with %\"PRIuMAX\" sub-entries\\n\",\n+\t\t s, (uintmax_t)me->namemap.nr);\n \tdebug_mm(\"mailmap: - simple: '%s' <%s>\\n\",\n \t\t debug_str(me->name), debug_str(me->email));\n \n@@ -250,7 +250,8 @@ int read_mailmap(struct string_list *map)\n \n void clear_mailmap(struct string_list *map)\n {\n-\tdebug_mm(\"mailmap: clearing %d entries...\\n\", map->nr);\n+\tdebug_mm(\"mailmap: clearing %\"PRIuMAX\" entries...\\n\",\n+\t\t (uintmax_t)map->nr);\n \tmap->strdup_strings = 1;\n \tstring_list_clear_func(map, free_mailmap_entry);\n \tdebug_mm(\"mailmap: cleared\\n\");\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 55decb2587e..bb2be2a2758 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4064,8 +4064,8 @@ static void process_entries(struct merge_options *opt,\n \ttrace2_region_enter(\"merge\", \"process_entries cleanup\", opt->repo);\n \tif (dir_metadata.offsets.nr != 1 ||\n \t    (uintptr_t)dir_metadata.offsets.items[0].util != 0) {\n-\t\tprintf(\"dir_metadata.offsets.nr = %d (should be 1)\\n\",\n-\t\t       dir_metadata.offsets.nr);\n+\t\tprintf(\"dir_metadata.offsets.nr = %\"PRIuMAX\" (should be 1)\\n\",\n+\t\t       (uintmax_t)dir_metadata.offsets.nr);\n \t\tprintf(\"dir_metadata.offsets.items[0].util = %u (should be 0)\\n\",\n \t\t       (unsigned)(uintptr_t)dir_metadata.offsets.items[0].util);\n \t\tfflush(stdout);\ndiff --git a/string-list.h b/string-list.h\nindex 267d6e5769d..d5a744e1438 100644\n--- a/string-list.h\n+++ b/string-list.h\n@@ -86,7 +86,8 @@ typedef int (*compare_strings_fn)(const char *, const char *);\n  */\n struct string_list {\n \tstruct string_list_item *items;\n-\tunsigned int nr, alloc;\n+\tsize_t nr;\n+\tsize_t alloc;\n \tunsigned int strdup_strings:1;\n \tcompare_strings_fn cmp; /* NULL uses strcmp() */\n };\ndiff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c\nindex 913775a14b7..eabd28defc5 100644\n--- a/t/helper/test-run-command.c\n+++ b/t/helper/test-run-command.c\n@@ -180,15 +180,16 @@ static int testsuite(int argc, const char **argv)\n \tif (max_jobs > suite.tests.nr)\n \t\tmax_jobs = suite.tests.nr;\n \n-\tfprintf(stderr, \"Running %d tests (%d at a time)\\n\",\n-\t\tsuite.tests.nr, max_jobs);\n+\tfprintf(stderr, \"Running %\"PRIuMAX\" tests (%d at a time)\\n\",\n+\t\t(uintmax_t)suite.tests.nr, max_jobs);\n \n \tret = run_processes_parallel(max_jobs, next_test, test_failed,\n \t\t\t\t     test_finished, &suite);\n \n \tif (suite.failed.nr > 0) {\n \t\tret = 1;\n-\t\tfprintf(stderr, \"%d tests failed:\\n\\n\", suite.failed.nr);\n+\t\tfprintf(stderr, \"%\"PRIuMAX\" tests failed:\\n\\n\",\n+\t\t\t(uintmax_t)suite.failed.nr);\n \t\tfor (i = 0; i < suite.failed.nr; i++)\n \t\t\tfprintf(stderr, \"\\t%s\\n\", suite.failed.items[i].string);\n \t}\ndiff --git a/wt-status.c b/wt-status.c\nindex 335e723a71e..b3d398d7864 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1374,10 +1374,10 @@ static void show_rebase_information(struct wt_status *s,\n \t\t\tstatus_printf_ln(s, color, _(\"No commands done.\"));\n \t\telse {\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\tQ_(\"Last command done (%d command done):\",\n-\t\t\t\t\t\"Last commands done (%d commands done):\",\n-\t\t\t\t\thave_done.nr),\n-\t\t\t\thave_done.nr);\n+\t\t\t\tQ_(\"Last command done (%\"PRIuMAX\" command done):\",\n+\t\t\t\t\t\"Last commands done (%\"PRIuMAX\" commands done):\",\n+\t\t\t\t\t(unsigned long)have_done.nr),\n+\t\t\t\t(uintmax_t)have_done.nr);\n \t\t\tfor (i = (have_done.nr > nr_lines_to_show)\n \t\t\t\t? have_done.nr - nr_lines_to_show : 0;\n \t\t\t\ti < have_done.nr;\n@@ -1393,10 +1393,10 @@ static void show_rebase_information(struct wt_status *s,\n \t\t\t\t\t _(\"No commands remaining.\"));\n \t\telse {\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\tQ_(\"Next command to do (%d remaining command):\",\n-\t\t\t\t\t\"Next commands to do (%d remaining commands):\",\n-\t\t\t\t\tyet_to_do.nr),\n-\t\t\t\tyet_to_do.nr);\n+\t\t\t\tQ_(\"Next command to do (%\"PRIuMAX\" remaining command):\",\n+\t\t\t\t\t\"Next commands to do (%\"PRIuMAX\" remaining commands):\",\n+\t\t\t\t\t(unsigned long)yet_to_do.nr),\n+\t\t\t\t(uintmax_t)yet_to_do.nr);\n \t\t\tfor (i = 0; i < nr_lines_to_show && i < yet_to_do.nr; i++)\n \t\t\t\tstatus_printf_ln(s, color, \"   %s\", yet_to_do.items[i].string);\n \t\t\tif (s->hints)\n-- \n2.35.1.1241.gd8d69a17521\n\n"},{"id":"450569","messageId":"a34c37c0-b6fd-9f4f-e990-9869f375937e@github.com","threadId":"57514","inReplyTo":"patch-1.2-83659fbc459-20220307T113707Z-avarab@gmail.com","subject":"Re: [PATCH 1/2] gettext API users: correct use of casts for Q_()","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-07T13:41:48Z","receivedAt":"2022-03-07T13:42:03Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/7/2022 6:38 AM, Ævar Arnfjörð Bjarmason wrote:\n> Change users of the inline gettext.h Q_() function to cast its\n> argument to \"unsigned long\" instead of \"int\" or \"unsigned int\".\n> \n> The ngettext() function (which Q_() resolves to) takes an \"unsigned\n> long int\", and so does our Q_() wrapper for it, see 0c9ea33b90f (i18n:\n> add stub Q_() wrapper for ngettext, 2011-03-09).\n> \n> In a subsequent commit we'll be making more use of this pattern of:\n> \n>     func(Q_(..%\"PRIuMAX..., (unsigned long)x), (uintmax_t)x);\n> \n> By making this change we ensure that this case isn't the odd one out\n> in that post-image.\n\n\n>  \tif (!res)\n> -\t\tprintf(Q_(\"updated %d path\\n\",\n> -\t\t\t  \"updated %d paths\\n\", count), (int)count);\n> +\t\tprintf(Q_(\"updated %\"PRIuMAX\" path\\n\",\n> +\t\t\t  \"updated %\"PRIuMAX\" paths\\n\", (unsigned long)count),\n> +\t\t       (uintmax_t)count);\n\nWhy are we adding more uses of \"unsigned long\" which is not consistent\nin its size across 64-bit Linux and 64-bit Windows? Specifically, on\nWindows \"unsigned long\" is _not_ uintmax_t. Shouldn't we be using\nuintmax_t everywhere instead?\n\nThanks,\n-Stolee\n"},{"id":"450570","messageId":"34eb7b5a-180c-0e4a-0929-8eaf52f80090@github.com","threadId":"57514","inReplyTo":"patch-2.2-398682c07aa-20220307T113707Z-avarab@gmail.com","subject":"Re: [PATCH 2/2] string-list API: change \"nr\" and \"alloc\" to \"size_t\"","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-07T13:43:32Z","receivedAt":"2022-03-07T13:43:44Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/7/2022 6:38 AM, Ævar Arnfjörð Bjarmason wrote:\n> Change the \"nr\" and \"alloc\" members of \"struct string_list\" to use\n> \"size_t\" instead of \"nr\". On some platforms the size of an \"unsigned\n> int\" will be smaller than a \"size_t\", e.g. a 32 bit unsigned v.s. 64\n> bit unsigned. As \"struct string_list\" is a generic API we use in a lot\n> of places this might cause overflows.\n> \n\n>  \t\t\tprintf_ln(Q_(\"The bundle requires this ref:\",\n> -\t\t\t\t     \"The bundle requires these %d refs:\",\n> -\t\t\t\t     r->nr),\n> -\t\t\t\t  r->nr);\n> +\t\t\t\t     \"The bundle requires these %\"PRIuMAX\" refs:\",\n> +\t\t\t\t     (unsigned long)r->nr),\n> +\t\t\t\t  (uintmax_t)r->nr);\n\nThere are more additions of unsigned long here, which will possibly\ntruncate the size_t of r->nr. I must be missing something here that\nexplains why you are making this choice.\n\nThanks,\n-Stolee\n"},{"id":"450578","messageId":"220307.86wnh5zwe0.gmgdl@evledraar.gmail.com","threadId":"57514","inReplyTo":"a34c37c0-b6fd-9f4f-e990-9869f375937e@github.com","subject":"Re: [PATCH 1/2] gettext API users: correct use of casts for Q_()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T13:54:51Z","receivedAt":"2022-03-07T14:10:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Mar 07 2022, Derrick Stolee wrote:\n\n> On 3/7/2022 6:38 AM, Ævar Arnfjörð Bjarmason wrote:\n>> Change users of the inline gettext.h Q_() function to cast its\n>> argument to \"unsigned long\" instead of \"int\" or \"unsigned int\".\n>> \n>> The ngettext() function (which Q_() resolves to) takes an \"unsigned\n>> long int\", and so does our Q_() wrapper for it, see 0c9ea33b90f (i18n:\n>> add stub Q_() wrapper for ngettext, 2011-03-09).\n>> \n>> In a subsequent commit we'll be making more use of this pattern of:\n>> \n>>     func(Q_(..%\"PRIuMAX..., (unsigned long)x), (uintmax_t)x);\n>> \n>> By making this change we ensure that this case isn't the odd one out\n>> in that post-image.\n>\n>\n>>  \tif (!res)\n>> -\t\tprintf(Q_(\"updated %d path\\n\",\n>> -\t\t\t  \"updated %d paths\\n\", count), (int)count);\n>> +\t\tprintf(Q_(\"updated %\"PRIuMAX\" path\\n\",\n>> +\t\t\t  \"updated %\"PRIuMAX\" paths\\n\", (unsigned long)count),\n>> +\t\t       (uintmax_t)count);\n>\n> Why are we adding more uses of \"unsigned long\" which is not consistent\n> in its size across 64-bit Linux and 64-bit Windows? Specifically, on\n> Windows \"unsigned long\" is _not_ uintmax_t. Shouldn't we be using\n> uintmax_t everywhere instead?\n\nWhatever we do with \"unsigned long\" v.s. \"size_t\" or \"uintmax_t\" here\nwe'll need to call the ngettext() function, which takes \"unsigned long\".\n\nSince you're quoting the part of the commit message that's explaining\nthat I'm not sure if you're meaning this as a suggestion that the\nexplanation should be clearer/more explicit, or just missed that\nngettext() isn't ours...\n\nI did wonder if we should just skip the casts here and instead do:\n\t\n\tdiff --git a/gettext.h b/gettext.h\n\tindex d209911ebb8..095bf6b0e5e 100644\n\t--- a/gettext.h\n\t+++ b/gettext.h\n\t@@ -49,8 +49,10 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)\n\t }\n\t \n\t static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)\n\t-const char *Q_(const char *msgid, const char *plu, unsigned long n)\n\t+const char *Q_(const char *msgid, const char *plu, size_t n)\n\t {\n\t+\tif (n > ULONG_MAX)\n\t+\t\tn = ULONG_MAX;\n\t \treturn ngettext(msgid, plu, n);\n\t }\n\nWhich I suppose would be more correct than a cast, but the edge case\nwhere we'd need that ULONG_MAX is so obscure that I don't think it's\nworth worrying about it.\n\nI think for this series it probably makes more sense to drop the casts\nfor the \"n\" argument entirely, what do you think?\n"},{"id":"450579","messageId":"220307.86sfrtzwdg.gmgdl@evledraar.gmail.com","threadId":"57514","inReplyTo":"34eb7b5a-180c-0e4a-0929-8eaf52f80090@github.com","subject":"Re: [PATCH 2/2] string-list API: change \"nr\" and \"alloc\" to \"size_t\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T14:10:23Z","receivedAt":"2022-03-07T14:10:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Mar 07 2022, Derrick Stolee wrote:\n\n> On 3/7/2022 6:38 AM, Ævar Arnfjörð Bjarmason wrote:\n>> Change the \"nr\" and \"alloc\" members of \"struct string_list\" to use\n>> \"size_t\" instead of \"nr\". On some platforms the size of an \"unsigned\n>> int\" will be smaller than a \"size_t\", e.g. a 32 bit unsigned v.s. 64\n>> bit unsigned. As \"struct string_list\" is a generic API we use in a lot\n>> of places this might cause overflows.\n>> \n>\n>>  \t\t\tprintf_ln(Q_(\"The bundle requires this ref:\",\n>> -\t\t\t\t     \"The bundle requires these %d refs:\",\n>> -\t\t\t\t     r->nr),\n>> -\t\t\t\t  r->nr);\n>> +\t\t\t\t     \"The bundle requires these %\"PRIuMAX\" refs:\",\n>> +\t\t\t\t     (unsigned long)r->nr),\n>> +\t\t\t\t  (uintmax_t)r->nr);\n>\n> There are more additions of unsigned long here, which will possibly\n> truncate the size_t of r->nr. I must be missing something here that\n> explains why you are making this choice.\n\nReplied to in the reply to your comment on 1/2.\n"},{"id":"450586","messageId":"cover-v2-0.2-00000000000-20220307T152316Z-avarab@gmail.com","threadId":"57514","inReplyTo":"cover-0.2-00000000000-20220307T113707Z-avarab@gmail.com","subject":"[PATCH v2 0/2] string-list.h: make \"nr\" and \"alloc\" a \"size_t\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T15:27:06Z","receivedAt":"2022-03-07T15:27:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"struct string_list\" to have a \"size_t\" for \"nr\" and\n\"alloc\", ensuring we won't overflow on platforms where size_t is 64\nbit, but \"unsigned int\" is 32 bit.\n\nThis replaces the v1 1/2 to get rid of the casts we use for Q_(), and\nthe 2/2's addition of casts is then consistent with those.\n\nÆvar Arnfjörð Bjarmason (2):\n  gettext API users: don't explicitly cast ngettext()'s \"n\"\n  string-list API: change \"nr\" and \"alloc\" to \"size_t\"\n\n builtin/index-pack.c        |  2 +-\n builtin/merge-recursive.c   |  2 +-\n builtin/receive-pack.c      |  9 +++++----\n builtin/shortlog.c          | 10 +++++-----\n bundle.c                    |  8 ++++----\n commit-graph.c              |  6 +++---\n mailmap.c                   |  7 ++++---\n merge-ort.c                 |  4 ++--\n strbuf.c                    |  4 ++--\n string-list.h               |  3 ++-\n t/helper/test-run-command.c |  7 ++++---\n wt-status.c                 | 12 ++++++------\n 12 files changed, 39 insertions(+), 35 deletions(-)\n\nRange-diff against v1:\n1:  83659fbc459 < -:  ----------- gettext API users: correct use of casts for Q_()\n-:  ----------- > 1:  83fd21741ad gettext API users: don't explicitly cast ngettext()'s \"n\"\n2:  398682c07aa ! 2:  6db8ab7a121 string-list API: change \"nr\" and \"alloc\" to \"size_t\"\n    @@ Commit message\n         two lines, which is the case for most such struct member\n         declarations (e.g. in \"strbuf.h\" and \"strvec.h\").\n     \n    +    Changing e.g. \"int i\" to \"size_t i\" in run_and_feed_hook() isn't\n    +    strictly necessary, and there are a lot more cases where we'll use a\n    +    local \"int\", \"unsigned int\" etc. variable derived from the \"nr\" in the\n    +    \"struct string_list\". But in that case as well as\n    +    add_wrapped_shortlog_msg() in builtin/shortlog.c we need to adjust the\n    +    printf format referring to \"nr\" anyway, so let's also change the other\n    +    variables referring to it.\n    +\n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n      ## builtin/receive-pack.c ##\n    @@ bundle.c: int verify_bundle(struct repository *r,\n      \t\tr = &header->references;\n      \t\tprintf_ln(Q_(\"The bundle contains this ref:\",\n     -\t\t\t     \"The bundle contains these %d refs:\",\n    --\t\t\t     r->nr),\n    --\t\t\t  r->nr);\n     +\t\t\t     \"The bundle contains these %\"PRIuMAX\" refs:\",\n    -+\t\t\t     (unsigned long)r->nr),\n    + \t\t\t     r->nr),\n    +-\t\t\t  r->nr);\n     +\t\t\t  (uintmax_t)r->nr);\n      \t\tlist_refs(r, 0, NULL);\n      \t\tr = &header->prerequisites;\n    @@ bundle.c: int verify_bundle(struct repository *r,\n      \t\t} else {\n      \t\t\tprintf_ln(Q_(\"The bundle requires this ref:\",\n     -\t\t\t\t     \"The bundle requires these %d refs:\",\n    --\t\t\t\t     r->nr),\n    --\t\t\t\t  r->nr);\n     +\t\t\t\t     \"The bundle requires these %\"PRIuMAX\" refs:\",\n    -+\t\t\t\t     (unsigned long)r->nr),\n    + \t\t\t\t     r->nr),\n    +-\t\t\t\t  r->nr);\n     +\t\t\t\t  (uintmax_t)r->nr);\n      \t\t\tlist_refs(r, 0, NULL);\n      \t\t}\n    @@ commit-graph.c: static int fill_oids_from_packs(struct write_commit_graph_contex\n      \t\tstrbuf_addf(&progress_title,\n     -\t\t\t    Q_(\"Finding commits for commit graph in %d pack\",\n     -\t\t\t       \"Finding commits for commit graph in %d packs\",\n    --\t\t\t       pack_indexes->nr),\n    --\t\t\t    pack_indexes->nr);\n     +\t\t\t    Q_(\"Finding commits for commit graph in %\"PRIuMAX\" pack\",\n     +\t\t\t       \"Finding commits for commit graph in %\"PRIuMAX\" packs\",\n    -+\t\t\t       (unsigned long)pack_indexes->nr),\n    + \t\t\t       pack_indexes->nr),\n    +-\t\t\t    pack_indexes->nr);\n     +\t\t\t    (uintmax_t)pack_indexes->nr);\n      \t\tctx->progress = start_delayed_progress(progress_title.buf, 0);\n      \t\tctx->progress_done = 0;\n    @@ wt-status.c: static void show_rebase_information(struct wt_status *s,\n      \t\t\tstatus_printf_ln(s, color,\n     -\t\t\t\tQ_(\"Last command done (%d command done):\",\n     -\t\t\t\t\t\"Last commands done (%d commands done):\",\n    --\t\t\t\t\thave_done.nr),\n    --\t\t\t\thave_done.nr);\n     +\t\t\t\tQ_(\"Last command done (%\"PRIuMAX\" command done):\",\n     +\t\t\t\t\t\"Last commands done (%\"PRIuMAX\" commands done):\",\n    -+\t\t\t\t\t(unsigned long)have_done.nr),\n    + \t\t\t\t\thave_done.nr),\n    +-\t\t\t\thave_done.nr);\n     +\t\t\t\t(uintmax_t)have_done.nr);\n      \t\t\tfor (i = (have_done.nr > nr_lines_to_show)\n      \t\t\t\t? have_done.nr - nr_lines_to_show : 0;\n    @@ wt-status.c: static void show_rebase_information(struct wt_status *s,\n      \t\t\tstatus_printf_ln(s, color,\n     -\t\t\t\tQ_(\"Next command to do (%d remaining command):\",\n     -\t\t\t\t\t\"Next commands to do (%d remaining commands):\",\n    --\t\t\t\t\tyet_to_do.nr),\n    --\t\t\t\tyet_to_do.nr);\n     +\t\t\t\tQ_(\"Next command to do (%\"PRIuMAX\" remaining command):\",\n     +\t\t\t\t\t\"Next commands to do (%\"PRIuMAX\" remaining commands):\",\n    -+\t\t\t\t\t(unsigned long)yet_to_do.nr),\n    + \t\t\t\t\tyet_to_do.nr),\n    +-\t\t\t\tyet_to_do.nr);\n     +\t\t\t\t(uintmax_t)yet_to_do.nr);\n      \t\t\tfor (i = 0; i < nr_lines_to_show && i < yet_to_do.nr; i++)\n      \t\t\t\tstatus_printf_ln(s, color, \"   %s\", yet_to_do.items[i].string);\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450587","messageId":"patch-v2-1.2-83fd21741ad-20220307T152316Z-avarab@gmail.com","threadId":"57514","inReplyTo":"cover-v2-0.2-00000000000-20220307T152316Z-avarab@gmail.com","subject":"[PATCH v2 1/2] gettext API users: don't explicitly cast ngettext()'s \"n\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T15:27:07Z","receivedAt":"2022-03-07T15:27:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a few stray users of the inline gettext.h Q_() function to stop\ncasting its \"n\" argument, the vast majority of the users of that\nwrapper API use the implicit cast to \"unsigned long\".\n\nThe ngettext() function (which Q_() resolves to) takes an \"unsigned\nlong int\", and so does our Q_() wrapper for it, see 0c9ea33b90f (i18n:\nadd stub Q_() wrapper for ngettext, 2011-03-09). The function isn't\nours, but provided by e.g. GNU libintl.\n\nThis amends code added in added in 7171a0b0cf5 (index-pack: correct\n\"len\" type in unpack_data(), 2016-07-13). The cast it added for the\nprintf format to die() was needed, but not the cast to Q_().\n\nLikewise the casts in strbuf.c added in 8f354a1faed (l10n: localizable\nupload progress messages, 2019-07-02) and for\nbuiltin/merge-recursive.c in ccf7813139f (i18n: merge-recursive: mark\nerror messages for translation, 2016-09-15) weren't needed.\n\nIn the latter case the cast was copy/pasted from the argument to\nwarning() itself, added in b74d779bd90 (MinGW: Fix compiler warning in\nmerge-recursive, 2009-05-23). The cast for warning() is needed, but\nnot the one for ngettext()'s \"n\" argument.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/index-pack.c      | 2 +-\n builtin/merge-recursive.c | 2 +-\n strbuf.c                  | 4 ++--\n 3 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 3c2e6aee3cc..89339e83a0d 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -579,7 +579,7 @@ static void *unpack_data(struct object_entry *obj,\n \t\tif (!n)\n \t\t\tdie(Q_(\"premature end of pack file, %\"PRIuMAX\" byte missing\",\n \t\t\t       \"premature end of pack file, %\"PRIuMAX\" bytes missing\",\n-\t\t\t       (unsigned int)len),\n+\t\t\t       len),\n \t\t\t    (uintmax_t)len);\n \t\tfrom += n;\n \t\tlen -= n;\ndiff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c\nindex a4bfd8fc51d..b9acbf5d342 100644\n--- a/builtin/merge-recursive.c\n+++ b/builtin/merge-recursive.c\n@@ -58,7 +58,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\t\t\t   \"Ignoring %s.\",\n \t\t\t\t   \"cannot handle more than %d bases. \"\n \t\t\t\t   \"Ignoring %s.\",\n-\t\t\t\t    (int)ARRAY_SIZE(bases)-1),\n+\t\t\t\t    ARRAY_SIZE(bases)-1),\n \t\t\t\t(int)ARRAY_SIZE(bases)-1, argv[i]);\n \t}\n \tif (argc - i != 3) /* \"--\" \"<head>\" \"<remote>\" */\ndiff --git a/strbuf.c b/strbuf.c\nindex 00abeb55afd..dd9eb85527a 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -875,9 +875,9 @@ static void strbuf_humanise(struct strbuf *buf, off_t bytes,\n \t\tstrbuf_addf(buf,\n \t\t\t\thumanise_rate == 0 ?\n \t\t\t\t\t/* TRANSLATORS: IEC 80000-13:2008 byte */\n-\t\t\t\t\tQ_(\"%u byte\", \"%u bytes\", (unsigned)bytes) :\n+\t\t\t\t\tQ_(\"%u byte\", \"%u bytes\", bytes) :\n \t\t\t\t\t/* TRANSLATORS: IEC 80000-13:2008 byte/second */\n-\t\t\t\t\tQ_(\"%u byte/s\", \"%u bytes/s\", (unsigned)bytes),\n+\t\t\t\t\tQ_(\"%u byte/s\", \"%u bytes/s\", bytes),\n \t\t\t\t(unsigned)bytes);\n \t}\n }\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450588","messageId":"patch-v2-2.2-6db8ab7a121-20220307T152316Z-avarab@gmail.com","threadId":"57514","inReplyTo":"cover-v2-0.2-00000000000-20220307T152316Z-avarab@gmail.com","subject":"[PATCH v2 2/2] string-list API: change \"nr\" and \"alloc\" to \"size_t\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T15:27:08Z","receivedAt":"2022-03-07T15:27:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"nr\" and \"alloc\" members of \"struct string_list\" to use\n\"size_t\" instead of \"nr\". On some platforms the size of an \"unsigned\nint\" will be smaller than a \"size_t\", e.g. a 32 bit unsigned v.s. 64\nbit unsigned. As \"struct string_list\" is a generic API we use in a lot\nof places this might cause overflows.\n\nAs one example: code in \"refs.c\" keeps track of the number of refs\nwith a \"size_t\", and auxiliary code in builtin/remote.c in\nget_ref_states() appends those to a \"struct string_list\".\n\nWhile we're at it split the \"nr\" and \"alloc\" in string-list.h across\ntwo lines, which is the case for most such struct member\ndeclarations (e.g. in \"strbuf.h\" and \"strvec.h\").\n\nChanging e.g. \"int i\" to \"size_t i\" in run_and_feed_hook() isn't\nstrictly necessary, and there are a lot more cases where we'll use a\nlocal \"int\", \"unsigned int\" etc. variable derived from the \"nr\" in the\n\"struct string_list\". But in that case as well as\nadd_wrapped_shortlog_msg() in builtin/shortlog.c we need to adjust the\nprintf format referring to \"nr\" anyway, so let's also change the other\nvariables referring to it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/receive-pack.c      |  9 +++++----\n builtin/shortlog.c          | 10 +++++-----\n bundle.c                    |  8 ++++----\n commit-graph.c              |  6 +++---\n mailmap.c                   |  7 ++++---\n merge-ort.c                 |  4 ++--\n string-list.h               |  3 ++-\n t/helper/test-run-command.c |  7 ++++---\n wt-status.c                 | 12 ++++++------\n 9 files changed, 35 insertions(+), 31 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex d10aeb7e78f..fc948a27c4f 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -813,13 +813,14 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed,\n \tproc.trace2_hook_name = hook_name;\n \n \tif (feed_state->push_options) {\n-\t\tint i;\n+\t\tsize_t i;\n \t\tfor (i = 0; i < feed_state->push_options->nr; i++)\n \t\t\tstrvec_pushf(&proc.env_array,\n-\t\t\t\t     \"GIT_PUSH_OPTION_%d=%s\", i,\n+\t\t\t\t     \"GIT_PUSH_OPTION_%\"PRIuMAX\"=%s\",\n+\t\t\t\t     (uintmax_t)i,\n \t\t\t\t     feed_state->push_options->items[i].string);\n-\t\tstrvec_pushf(&proc.env_array, \"GIT_PUSH_OPTION_COUNT=%d\",\n-\t\t\t     feed_state->push_options->nr);\n+\t\tstrvec_pushf(&proc.env_array, \"GIT_PUSH_OPTION_COUNT=%\"PRIuMAX\"\",\n+\t\t\t     (uintmax_t)feed_state->push_options->nr);\n \t} else\n \t\tstrvec_pushf(&proc.env_array, \"GIT_PUSH_OPTION_COUNT\");\n \ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 228d782754a..26c5c0cf935 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -435,7 +435,7 @@ static void add_wrapped_shortlog_msg(struct strbuf *sb, const char *s,\n \n void shortlog_output(struct shortlog *log)\n {\n-\tint i, j;\n+\tsize_t i, j;\n \tstruct strbuf sb = STRBUF_INIT;\n \n \tif (log->sort_by_number)\n@@ -448,10 +448,10 @@ void shortlog_output(struct shortlog *log)\n \t\t\t\t(int)UTIL_TO_INT(item), item->string);\n \t\t} else {\n \t\t\tstruct string_list *onelines = item->util;\n-\t\t\tfprintf(log->file, \"%s (%d):\\n\",\n-\t\t\t\titem->string, onelines->nr);\n-\t\t\tfor (j = onelines->nr - 1; j >= 0; j--) {\n-\t\t\t\tconst char *msg = onelines->items[j].string;\n+\t\t\tfprintf(log->file, \"%s (%\"PRIuMAX\"):\\n\",\n+\t\t\t\titem->string, (uintmax_t)onelines->nr);\n+\t\t\tfor (j = onelines->nr; j >= 1; j--) {\n+\t\t\t\tconst char *msg = onelines->items[j - 1].string;\n \n \t\t\t\tif (log->wrap_lines) {\n \t\t\t\t\tstrbuf_reset(&sb);\ndiff --git a/bundle.c b/bundle.c\nindex a0bb687b0f4..7608701a51e 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -255,18 +255,18 @@ int verify_bundle(struct repository *r,\n \n \t\tr = &header->references;\n \t\tprintf_ln(Q_(\"The bundle contains this ref:\",\n-\t\t\t     \"The bundle contains these %d refs:\",\n+\t\t\t     \"The bundle contains these %\"PRIuMAX\" refs:\",\n \t\t\t     r->nr),\n-\t\t\t  r->nr);\n+\t\t\t  (uintmax_t)r->nr);\n \t\tlist_refs(r, 0, NULL);\n \t\tr = &header->prerequisites;\n \t\tif (!r->nr) {\n \t\t\tprintf_ln(_(\"The bundle records a complete history.\"));\n \t\t} else {\n \t\t\tprintf_ln(Q_(\"The bundle requires this ref:\",\n-\t\t\t\t     \"The bundle requires these %d refs:\",\n+\t\t\t\t     \"The bundle requires these %\"PRIuMAX\" refs:\",\n \t\t\t\t     r->nr),\n-\t\t\t\t  r->nr);\n+\t\t\t\t  (uintmax_t)r->nr);\n \t\t\tlist_refs(r, 0, NULL);\n \t\t}\n \t}\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 265c010122e..e7731db8f40 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1690,10 +1690,10 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,\n \tdirlen = packname.len;\n \tif (ctx->report_progress) {\n \t\tstrbuf_addf(&progress_title,\n-\t\t\t    Q_(\"Finding commits for commit graph in %d pack\",\n-\t\t\t       \"Finding commits for commit graph in %d packs\",\n+\t\t\t    Q_(\"Finding commits for commit graph in %\"PRIuMAX\" pack\",\n+\t\t\t       \"Finding commits for commit graph in %\"PRIuMAX\" packs\",\n \t\t\t       pack_indexes->nr),\n-\t\t\t    pack_indexes->nr);\n+\t\t\t    (uintmax_t)pack_indexes->nr);\n \t\tctx->progress = start_delayed_progress(progress_title.buf, 0);\n \t\tctx->progress_done = 0;\n \t}\ndiff --git a/mailmap.c b/mailmap.c\nindex 40ce152024d..7befdc5e483 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -43,8 +43,8 @@ static void free_mailmap_info(void *p, const char *s)\n static void free_mailmap_entry(void *p, const char *s)\n {\n \tstruct mailmap_entry *me = (struct mailmap_entry *)p;\n-\tdebug_mm(\"mailmap: removing entries for <%s>, with %d sub-entries\\n\",\n-\t\t s, me->namemap.nr);\n+\tdebug_mm(\"mailmap: removing entries for <%s>, with %\"PRIuMAX\" sub-entries\\n\",\n+\t\t s, (uintmax_t)me->namemap.nr);\n \tdebug_mm(\"mailmap: - simple: '%s' <%s>\\n\",\n \t\t debug_str(me->name), debug_str(me->email));\n \n@@ -250,7 +250,8 @@ int read_mailmap(struct string_list *map)\n \n void clear_mailmap(struct string_list *map)\n {\n-\tdebug_mm(\"mailmap: clearing %d entries...\\n\", map->nr);\n+\tdebug_mm(\"mailmap: clearing %\"PRIuMAX\" entries...\\n\",\n+\t\t (uintmax_t)map->nr);\n \tmap->strdup_strings = 1;\n \tstring_list_clear_func(map, free_mailmap_entry);\n \tdebug_mm(\"mailmap: cleared\\n\");\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 55decb2587e..bb2be2a2758 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4064,8 +4064,8 @@ static void process_entries(struct merge_options *opt,\n \ttrace2_region_enter(\"merge\", \"process_entries cleanup\", opt->repo);\n \tif (dir_metadata.offsets.nr != 1 ||\n \t    (uintptr_t)dir_metadata.offsets.items[0].util != 0) {\n-\t\tprintf(\"dir_metadata.offsets.nr = %d (should be 1)\\n\",\n-\t\t       dir_metadata.offsets.nr);\n+\t\tprintf(\"dir_metadata.offsets.nr = %\"PRIuMAX\" (should be 1)\\n\",\n+\t\t       (uintmax_t)dir_metadata.offsets.nr);\n \t\tprintf(\"dir_metadata.offsets.items[0].util = %u (should be 0)\\n\",\n \t\t       (unsigned)(uintptr_t)dir_metadata.offsets.items[0].util);\n \t\tfflush(stdout);\ndiff --git a/string-list.h b/string-list.h\nindex 267d6e5769d..d5a744e1438 100644\n--- a/string-list.h\n+++ b/string-list.h\n@@ -86,7 +86,8 @@ typedef int (*compare_strings_fn)(const char *, const char *);\n  */\n struct string_list {\n \tstruct string_list_item *items;\n-\tunsigned int nr, alloc;\n+\tsize_t nr;\n+\tsize_t alloc;\n \tunsigned int strdup_strings:1;\n \tcompare_strings_fn cmp; /* NULL uses strcmp() */\n };\ndiff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c\nindex 913775a14b7..eabd28defc5 100644\n--- a/t/helper/test-run-command.c\n+++ b/t/helper/test-run-command.c\n@@ -180,15 +180,16 @@ static int testsuite(int argc, const char **argv)\n \tif (max_jobs > suite.tests.nr)\n \t\tmax_jobs = suite.tests.nr;\n \n-\tfprintf(stderr, \"Running %d tests (%d at a time)\\n\",\n-\t\tsuite.tests.nr, max_jobs);\n+\tfprintf(stderr, \"Running %\"PRIuMAX\" tests (%d at a time)\\n\",\n+\t\t(uintmax_t)suite.tests.nr, max_jobs);\n \n \tret = run_processes_parallel(max_jobs, next_test, test_failed,\n \t\t\t\t     test_finished, &suite);\n \n \tif (suite.failed.nr > 0) {\n \t\tret = 1;\n-\t\tfprintf(stderr, \"%d tests failed:\\n\\n\", suite.failed.nr);\n+\t\tfprintf(stderr, \"%\"PRIuMAX\" tests failed:\\n\\n\",\n+\t\t\t(uintmax_t)suite.failed.nr);\n \t\tfor (i = 0; i < suite.failed.nr; i++)\n \t\t\tfprintf(stderr, \"\\t%s\\n\", suite.failed.items[i].string);\n \t}\ndiff --git a/wt-status.c b/wt-status.c\nindex 335e723a71e..bd54cf59894 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1374,10 +1374,10 @@ static void show_rebase_information(struct wt_status *s,\n \t\t\tstatus_printf_ln(s, color, _(\"No commands done.\"));\n \t\telse {\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\tQ_(\"Last command done (%d command done):\",\n-\t\t\t\t\t\"Last commands done (%d commands done):\",\n+\t\t\t\tQ_(\"Last command done (%\"PRIuMAX\" command done):\",\n+\t\t\t\t\t\"Last commands done (%\"PRIuMAX\" commands done):\",\n \t\t\t\t\thave_done.nr),\n-\t\t\t\thave_done.nr);\n+\t\t\t\t(uintmax_t)have_done.nr);\n \t\t\tfor (i = (have_done.nr > nr_lines_to_show)\n \t\t\t\t? have_done.nr - nr_lines_to_show : 0;\n \t\t\t\ti < have_done.nr;\n@@ -1393,10 +1393,10 @@ static void show_rebase_information(struct wt_status *s,\n \t\t\t\t\t _(\"No commands remaining.\"));\n \t\telse {\n \t\t\tstatus_printf_ln(s, color,\n-\t\t\t\tQ_(\"Next command to do (%d remaining command):\",\n-\t\t\t\t\t\"Next commands to do (%d remaining commands):\",\n+\t\t\t\tQ_(\"Next command to do (%\"PRIuMAX\" remaining command):\",\n+\t\t\t\t\t\"Next commands to do (%\"PRIuMAX\" remaining commands):\",\n \t\t\t\t\tyet_to_do.nr),\n-\t\t\t\tyet_to_do.nr);\n+\t\t\t\t(uintmax_t)yet_to_do.nr);\n \t\t\tfor (i = 0; i < nr_lines_to_show && i < yet_to_do.nr; i++)\n \t\t\t\tstatus_printf_ln(s, color, \"   %s\", yet_to_do.items[i].string);\n \t\t\tif (s->hints)\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450595","messageId":"03f493f2-aa94-ef9e-8b05-88a895015c80@github.com","threadId":"57514","inReplyTo":"220307.86wnh5zwe0.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 1/2] gettext API users: correct use of casts for Q_()","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-07T15:53:34Z","receivedAt":"2022-03-07T15:53:39Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/7/2022 8:54 AM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Mon, Mar 07 2022, Derrick Stolee wrote:\n> \n>> On 3/7/2022 6:38 AM, Ævar Arnfjörð Bjarmason wrote:\n>>> Change users of the inline gettext.h Q_() function to cast its\n>>> argument to \"unsigned long\" instead of \"int\" or \"unsigned int\".\n>>>\n>>> The ngettext() function (which Q_() resolves to) takes an \"unsigned\n>>> long int\", and so does our Q_() wrapper for it, see 0c9ea33b90f (i18n:\n>>> add stub Q_() wrapper for ngettext, 2011-03-09).\n>>>\n>>> In a subsequent commit we'll be making more use of this pattern of:\n>>>\n>>>     func(Q_(..%\"PRIuMAX..., (unsigned long)x), (uintmax_t)x);\n>>>\n>>> By making this change we ensure that this case isn't the odd one out\n>>> in that post-image.\n>>\n>>\n>>>  \tif (!res)\n>>> -\t\tprintf(Q_(\"updated %d path\\n\",\n>>> -\t\t\t  \"updated %d paths\\n\", count), (int)count);\n>>> +\t\tprintf(Q_(\"updated %\"PRIuMAX\" path\\n\",\n>>> +\t\t\t  \"updated %\"PRIuMAX\" paths\\n\", (unsigned long)count),\n>>> +\t\t       (uintmax_t)count);\n>>\n>> Why are we adding more uses of \"unsigned long\" which is not consistent\n>> in its size across 64-bit Linux and 64-bit Windows? Specifically, on\n>> Windows \"unsigned long\" is _not_ uintmax_t. Shouldn't we be using\n>> uintmax_t everywhere instead?\n> \n> Whatever we do with \"unsigned long\" v.s. \"size_t\" or \"uintmax_t\" here\n> we'll need to call the ngettext() function, which takes \"unsigned long\".\n> \n> Since you're quoting the part of the commit message that's explaining\n> that I'm not sure if you're meaning this as a suggestion that the\n> explanation should be clearer/more explicit, or just missed that\n> ngettext() isn't ours...\n\nMostly missed which parts we had control over or not.\n\n> I did wonder if we should just skip the casts here and instead do:\n> \t\n> \tdiff --git a/gettext.h b/gettext.h\n> \tindex d209911ebb8..095bf6b0e5e 100644\n> \t--- a/gettext.h\n> \t+++ b/gettext.h\n> \t@@ -49,8 +49,10 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)\n> \t }\n> \t \n> \t static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)\n> \t-const char *Q_(const char *msgid, const char *plu, unsigned long n)\n> \t+const char *Q_(const char *msgid, const char *plu, size_t n)\n> \t {\n> \t+\tif (n > ULONG_MAX)\n> \t+\t\tn = ULONG_MAX;\n> \t \treturn ngettext(msgid, plu, n);\n> \t }\n> \n> Which I suppose would be more correct than a cast, but the edge case\n> where we'd need that ULONG_MAX is so obscure that I don't think it's\n> worth worrying about it.\n> \n> I think for this series it probably makes more sense to drop the casts\n> for the \"n\" argument entirely, what do you think?\n\nI agree that this is a better approach. It avoids polluting all of\nthe callers with down-casts and instead puts it here. The only\ndifference is that we will truncate at ULONG_MAX instead of blindly\nignoring the significant bits, which is probably better.\n\nThanks,\n-Stolee\n"},{"id":"450596","messageId":"0b0802fb-c977-d8ab-092a-b8a576daf3c6@github.com","threadId":"57514","inReplyTo":"cover-v2-0.2-00000000000-20220307T152316Z-avarab@gmail.com","subject":"Re: [PATCH v2 0/2] string-list.h: make \"nr\" and \"alloc\" a \"size_t\"","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-07T15:55:02Z","receivedAt":"2022-03-07T15:55:07Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/7/2022 10:27 AM, Ævar Arnfjörð Bjarmason wrote:\n> Change the \"struct string_list\" to have a \"size_t\" for \"nr\" and\n> \"alloc\", ensuring we won't overflow on platforms where size_t is 64\n> bit, but \"unsigned int\" is 32 bit.\n> \n> This replaces the v1 1/2 to get rid of the casts we use for Q_(), and\n> the 2/2's addition of casts is then consistent with those.\n\nThis version looks a lot better, localizing the need for understanding\ncast implications into the implementation of Q_() instead of every\ncaller.\n\nThanks,\n-Stolee\n"},{"id":"450605","messageId":"9c9ca557-122e-3bcb-db4b-3b5ba7bf9369@iee.email","threadId":"57514","inReplyTo":"patch-v2-2.2-6db8ab7a121-20220307T152316Z-avarab@gmail.com","subject":"Re: [PATCH v2 2/2] string-list API: change \"nr\" and \"alloc\" to \"size_t\"","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-03-07T16:23:07Z","receivedAt":"2022-03-07T16:23:15Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 07/03/2022 15:27, Ævar Arnfjörð Bjarmason wrote:\n> Change the \"nr\" and \"alloc\" members of \"struct string_list\" to use\n> \"size_t\" instead of \"nr\". On some platforms the size of an \"unsigned\n> int\" will be smaller than a \"size_t\", e.g. a 32 bit unsigned v.s. 64\n> bit unsigned. As \"struct string_list\" is a generic API we use in a lot\n> of places this might cause overflows.\n>\n> As one example: code in \"refs.c\" keeps track of the number of refs\n> with a \"size_t\", and auxiliary code in builtin/remote.c in\n> get_ref_states() appends those to a \"struct string_list\".\n>\n> While we're at it split the \"nr\" and \"alloc\" in string-list.h across\n> two lines, which is the case for most such struct member\n> declarations (e.g. in \"strbuf.h\" and \"strvec.h\").\n>\n> Changing e.g. \"int i\" to \"size_t i\" in run_and_feed_hook() isn't\n> strictly necessary, and there are a lot more cases where we'll use a\n> local \"int\", \"unsigned int\" etc. variable derived from the \"nr\" in the\n> \"struct string_list\". But in that case as well as\n> add_wrapped_shortlog_msg() in builtin/shortlog.c we need to adjust the\n> printf format referring to \"nr\" anyway, so let's also change the other\n> variables referring to it.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>  builtin/receive-pack.c      |  9 +++++----\n>  builtin/shortlog.c          | 10 +++++-----\n>  bundle.c                    |  8 ++++----\n>  commit-graph.c              |  6 +++---\n>  mailmap.c                   |  7 ++++---\n>  merge-ort.c                 |  4 ++--\n>  string-list.h               |  3 ++-\n>  t/helper/test-run-command.c |  7 ++++---\n>  wt-status.c                 | 12 ++++++------\n>  9 files changed, 35 insertions(+), 31 deletions(-)\n>\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index d10aeb7e78f..fc948a27c4f 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -813,13 +813,14 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed,\n>  \tproc.trace2_hook_name = hook_name;\n>  \n>  \tif (feed_state->push_options) {\n> -\t\tint i;\n> +\t\tsize_t i;\n>  \t\tfor (i = 0; i < feed_state->push_options->nr; i++)\n>  \t\t\tstrvec_pushf(&proc.env_array,\n> -\t\t\t\t     \"GIT_PUSH_OPTION_%d=%s\", i,\n> +\t\t\t\t     \"GIT_PUSH_OPTION_%\"PRIuMAX\"=%s\",\n> +\t\t\t\t     (uintmax_t)i,\n>  \t\t\t\t     feed_state->push_options->items[i].string);\n> -\t\tstrvec_pushf(&proc.env_array, \"GIT_PUSH_OPTION_COUNT=%d\",\n> -\t\t\t     feed_state->push_options->nr);\n> +\t\tstrvec_pushf(&proc.env_array, \"GIT_PUSH_OPTION_COUNT=%\"PRIuMAX\"\",\n> +\t\t\t     (uintmax_t)feed_state->push_options->nr);\n>  \t} else\n>  \t\tstrvec_pushf(&proc.env_array, \"GIT_PUSH_OPTION_COUNT\");\n>  \n> diff --git a/builtin/shortlog.c b/builtin/shortlog.c\n> index 228d782754a..26c5c0cf935 100644\n> --- a/builtin/shortlog.c\n> +++ b/builtin/shortlog.c\n> @@ -435,7 +435,7 @@ static void add_wrapped_shortlog_msg(struct strbuf *sb, const char *s,\n>  \n>  void shortlog_output(struct shortlog *log)\n>  {\n> -\tint i, j;\n> +\tsize_t i, j;\n>  \tstruct strbuf sb = STRBUF_INIT;\n>  \n>  \tif (log->sort_by_number)\n> @@ -448,10 +448,10 @@ void shortlog_output(struct shortlog *log)\n>  \t\t\t\t(int)UTIL_TO_INT(item), item->string);\n>  \t\t} else {\n>  \t\t\tstruct string_list *onelines = item->util;\n> -\t\t\tfprintf(log->file, \"%s (%d):\\n\",\n> -\t\t\t\titem->string, onelines->nr);\n> -\t\t\tfor (j = onelines->nr - 1; j >= 0; j--) {\n> -\t\t\t\tconst char *msg = onelines->items[j].string;\n> +\t\t\tfprintf(log->file, \"%s (%\"PRIuMAX\"):\\n\",\n> +\t\t\t\titem->string, (uintmax_t)onelines->nr);\n> +\t\t\tfor (j = onelines->nr; j >= 1; j--) {\n> +\t\t\t\tconst char *msg = onelines->items[j - 1].string;\nDoes this change of iteration limits and j's offset  need a commit\nmessage comment?\n\nPhilip\n\n>  \n>  \t\t\t\tif (log->wrap_lines) {\n>  \t\t\t\t\tstrbuf_reset(&sb);\n> diff --git a/bundle.c b/bundle.c\n> index a0bb687b0f4..7608701a51e 100644\n> --- a/bundle.c\n> +++ b/bundle.c\n> @@ -255,18 +255,18 @@ int verify_bundle(struct repository *r,\n>  \n>  \t\tr = &header->references;\n>  \t\tprintf_ln(Q_(\"The bundle contains this ref:\",\n> -\t\t\t     \"The bundle contains these %d refs:\",\n> +\t\t\t     \"The bundle contains these %\"PRIuMAX\" refs:\",\n>  \t\t\t     r->nr),\n> -\t\t\t  r->nr);\n> +\t\t\t  (uintmax_t)r->nr);\n>  \t\tlist_refs(r, 0, NULL);\n>  \t\tr = &header->prerequisites;\n>  \t\tif (!r->nr) {\n>  \t\t\tprintf_ln(_(\"The bundle records a complete history.\"));\n>  \t\t} else {\n>  \t\t\tprintf_ln(Q_(\"The bundle requires this ref:\",\n> -\t\t\t\t     \"The bundle requires these %d refs:\",\n> +\t\t\t\t     \"The bundle requires these %\"PRIuMAX\" refs:\",\n>  \t\t\t\t     r->nr),\n> -\t\t\t\t  r->nr);\n> +\t\t\t\t  (uintmax_t)r->nr);\n>  \t\t\tlist_refs(r, 0, NULL);\n>  \t\t}\n>  \t}\n> diff --git a/commit-graph.c b/commit-graph.c\n> index 265c010122e..e7731db8f40 100644\n> --- a/commit-graph.c\n> +++ b/commit-graph.c\n> @@ -1690,10 +1690,10 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,\n>  \tdirlen = packname.len;\n>  \tif (ctx->report_progress) {\n>  \t\tstrbuf_addf(&progress_title,\n> -\t\t\t    Q_(\"Finding commits for commit graph in %d pack\",\n> -\t\t\t       \"Finding commits for commit graph in %d packs\",\n> +\t\t\t    Q_(\"Finding commits for commit graph in %\"PRIuMAX\" pack\",\n> +\t\t\t       \"Finding commits for commit graph in %\"PRIuMAX\" packs\",\n>  \t\t\t       pack_indexes->nr),\n> -\t\t\t    pack_indexes->nr);\n> +\t\t\t    (uintmax_t)pack_indexes->nr);\n>  \t\tctx->progress = start_delayed_progress(progress_title.buf, 0);\n>  \t\tctx->progress_done = 0;\n>  \t}\n> diff --git a/mailmap.c b/mailmap.c\n> index 40ce152024d..7befdc5e483 100644\n> --- a/mailmap.c\n> +++ b/mailmap.c\n> @@ -43,8 +43,8 @@ static void free_mailmap_info(void *p, const char *s)\n>  static void free_mailmap_entry(void *p, const char *s)\n>  {\n>  \tstruct mailmap_entry *me = (struct mailmap_entry *)p;\n> -\tdebug_mm(\"mailmap: removing entries for <%s>, with %d sub-entries\\n\",\n> -\t\t s, me->namemap.nr);\n> +\tdebug_mm(\"mailmap: removing entries for <%s>, with %\"PRIuMAX\" sub-entries\\n\",\n> +\t\t s, (uintmax_t)me->namemap.nr);\n>  \tdebug_mm(\"mailmap: - simple: '%s' <%s>\\n\",\n>  \t\t debug_str(me->name), debug_str(me->email));\n>  \n> @@ -250,7 +250,8 @@ int read_mailmap(struct string_list *map)\n>  \n>  void clear_mailmap(struct string_list *map)\n>  {\n> -\tdebug_mm(\"mailmap: clearing %d entries...\\n\", map->nr);\n> +\tdebug_mm(\"mailmap: clearing %\"PRIuMAX\" entries...\\n\",\n> +\t\t (uintmax_t)map->nr);\n>  \tmap->strdup_strings = 1;\n>  \tstring_list_clear_func(map, free_mailmap_entry);\n>  \tdebug_mm(\"mailmap: cleared\\n\");\n> diff --git a/merge-ort.c b/merge-ort.c\n> index 55decb2587e..bb2be2a2758 100644\n> --- a/merge-ort.c\n> +++ b/merge-ort.c\n> @@ -4064,8 +4064,8 @@ static void process_entries(struct merge_options *opt,\n>  \ttrace2_region_enter(\"merge\", \"process_entries cleanup\", opt->repo);\n>  \tif (dir_metadata.offsets.nr != 1 ||\n>  \t    (uintptr_t)dir_metadata.offsets.items[0].util != 0) {\n> -\t\tprintf(\"dir_metadata.offsets.nr = %d (should be 1)\\n\",\n> -\t\t       dir_metadata.offsets.nr);\n> +\t\tprintf(\"dir_metadata.offsets.nr = %\"PRIuMAX\" (should be 1)\\n\",\n> +\t\t       (uintmax_t)dir_metadata.offsets.nr);\n>  \t\tprintf(\"dir_metadata.offsets.items[0].util = %u (should be 0)\\n\",\n>  \t\t       (unsigned)(uintptr_t)dir_metadata.offsets.items[0].util);\n>  \t\tfflush(stdout);\n> diff --git a/string-list.h b/string-list.h\n> index 267d6e5769d..d5a744e1438 100644\n> --- a/string-list.h\n> +++ b/string-list.h\n> @@ -86,7 +86,8 @@ typedef int (*compare_strings_fn)(const char *, const char *);\n>   */\n>  struct string_list {\n>  \tstruct string_list_item *items;\n> -\tunsigned int nr, alloc;\n> +\tsize_t nr;\n> +\tsize_t alloc;\n>  \tunsigned int strdup_strings:1;\n>  \tcompare_strings_fn cmp; /* NULL uses strcmp() */\n>  };\n> diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c\n> index 913775a14b7..eabd28defc5 100644\n> --- a/t/helper/test-run-command.c\n> +++ b/t/helper/test-run-command.c\n> @@ -180,15 +180,16 @@ static int testsuite(int argc, const char **argv)\n>  \tif (max_jobs > suite.tests.nr)\n>  \t\tmax_jobs = suite.tests.nr;\n>  \n> -\tfprintf(stderr, \"Running %d tests (%d at a time)\\n\",\n> -\t\tsuite.tests.nr, max_jobs);\n> +\tfprintf(stderr, \"Running %\"PRIuMAX\" tests (%d at a time)\\n\",\n> +\t\t(uintmax_t)suite.tests.nr, max_jobs);\n>  \n>  \tret = run_processes_parallel(max_jobs, next_test, test_failed,\n>  \t\t\t\t     test_finished, &suite);\n>  \n>  \tif (suite.failed.nr > 0) {\n>  \t\tret = 1;\n> -\t\tfprintf(stderr, \"%d tests failed:\\n\\n\", suite.failed.nr);\n> +\t\tfprintf(stderr, \"%\"PRIuMAX\" tests failed:\\n\\n\",\n> +\t\t\t(uintmax_t)suite.failed.nr);\n>  \t\tfor (i = 0; i < suite.failed.nr; i++)\n>  \t\t\tfprintf(stderr, \"\\t%s\\n\", suite.failed.items[i].string);\n>  \t}\n> diff --git a/wt-status.c b/wt-status.c\n> index 335e723a71e..bd54cf59894 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1374,10 +1374,10 @@ static void show_rebase_information(struct wt_status *s,\n>  \t\t\tstatus_printf_ln(s, color, _(\"No commands done.\"));\n>  \t\telse {\n>  \t\t\tstatus_printf_ln(s, color,\n> -\t\t\t\tQ_(\"Last command done (%d command done):\",\n> -\t\t\t\t\t\"Last commands done (%d commands done):\",\n> +\t\t\t\tQ_(\"Last command done (%\"PRIuMAX\" command done):\",\n> +\t\t\t\t\t\"Last commands done (%\"PRIuMAX\" commands done):\",\n>  \t\t\t\t\thave_done.nr),\n> -\t\t\t\thave_done.nr);\n> +\t\t\t\t(uintmax_t)have_done.nr);\n>  \t\t\tfor (i = (have_done.nr > nr_lines_to_show)\n>  \t\t\t\t? have_done.nr - nr_lines_to_show : 0;\n>  \t\t\t\ti < have_done.nr;\n> @@ -1393,10 +1393,10 @@ static void show_rebase_information(struct wt_status *s,\n>  \t\t\t\t\t _(\"No commands remaining.\"));\n>  \t\telse {\n>  \t\t\tstatus_printf_ln(s, color,\n> -\t\t\t\tQ_(\"Next command to do (%d remaining command):\",\n> -\t\t\t\t\t\"Next commands to do (%d remaining commands):\",\n> +\t\t\t\tQ_(\"Next command to do (%\"PRIuMAX\" remaining command):\",\n> +\t\t\t\t\t\"Next commands to do (%\"PRIuMAX\" remaining commands):\",\n>  \t\t\t\t\tyet_to_do.nr),\n> -\t\t\t\tyet_to_do.nr);\n> +\t\t\t\t(uintmax_t)yet_to_do.nr);\n>  \t\t\tfor (i = 0; i < nr_lines_to_show && i < yet_to_do.nr; i++)\n>  \t\t\t\tstatus_printf_ln(s, color, \"   %s\", yet_to_do.items[i].string);\n>  \t\t\tif (s->hints)\n\n"},{"id":"450645","messageId":"xmqqy21lqyrk.fsf@gitster.g","threadId":"57514","inReplyTo":"9c9ca557-122e-3bcb-db4b-3b5ba7bf9369@iee.email","subject":"Re: [PATCH v2 2/2] string-list API: change \"nr\" and \"alloc\" to \"size_t\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-07T20:43:43Z","receivedAt":"2022-03-07T20:43:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley <philipoakley@iee.email> writes:\n\n>> @@ -448,10 +448,10 @@ void shortlog_output(struct shortlog *log)\n>>  \t\t\t\t(int)UTIL_TO_INT(item), item->string);\n>>  \t\t} else {\n>>  \t\t\tstruct string_list *onelines = item->util;\n>> -\t\t\tfprintf(log->file, \"%s (%d):\\n\",\n>> -\t\t\t\titem->string, onelines->nr);\n>> -\t\t\tfor (j = onelines->nr - 1; j >= 0; j--) {\n>> -\t\t\t\tconst char *msg = onelines->items[j].string;\n>> +\t\t\tfprintf(log->file, \"%s (%\"PRIuMAX\"):\\n\",\n>> +\t\t\t\titem->string, (uintmax_t)onelines->nr);\n>> +\t\t\tfor (j = onelines->nr; j >= 1; j--) {\n>> +\t\t\t\tconst char *msg = onelines->items[j - 1].string;\n> Does this change of iteration limits and j's offset need a commit\n> message comment?\n\nGood eyes.  I also tried to made sure this is the only loop that\ncounts toward zero that use a variable whose type has become size_t\nwith this patch, which is unsigned and cannot use \"var >= 0\" as the\nloop termination condition, but what is not in the patch is hard to\nknow.  If there were loops that used to count down with the .nr or\nthe .alloc members of string_list structure, they would need to be\nupdated, but if such a code exists, it is likely a bug already.\n\nI think many internal index string-list.c uses are still \"int\"\n(cf. get_entry_index() and functions on the call graph that leads to\nit, like add_entry() and string_list_remove()) that wants to be\nupdated to \"ssize_t\"; if the string_list has very many elements,\nadd_entry() may not find an existing string and add a duplicate at\nthe end, because the bisection search get_entry_index() uses cannot\nmove the upper-bound pointer beyond \"int\".\n\nIt would have been better if you culled the parts of the patch you\nare not referring to, by the way.\n\nThanks.\n\n"},{"id":"450675","messageId":"473ef11a-38d5-3bb1-6965-21050d90ae96@iee.email","threadId":"57514","inReplyTo":"xmqqy21lqyrk.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] string-list API: change \"nr\" and \"alloc\" to \"size_t\"","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-03-07T23:34:35Z","receivedAt":"2022-03-07T23:34:40Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 07/03/2022 20:43, Junio C Hamano wrote:\n> It would have been better if you culled the parts of the patch you\n> are not referring to, by the way.\nOops sorry, I'm not that great at deciding which parts to keep/cull to\nensure sufficient context for other readers - it wasn't something that\nwas desired when I was working (too many 'Outlook' influences).\n\nI'll work on it.\n\nPhilip\n"}]}