{"thread":{"id":"58949","subject":"[PATCH] git: remove unneeded casts","startedAt":"2022-12-14T15:35:36Z","lastAt":"2022-12-19T01:07:53Z","messageCount":8,"participants":["Rose via GitGitGadget","Jeff King","Junio C Hamano","Phillip Wood","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"469016","messageId":"pull.1396.git.git.1671032126602.gitgitgadget@gmail.com","threadId":"58949","inReplyTo":null,"subject":"[PATCH] git: remove unneeded casts","fromName":"Rose via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-12-14T15:35:26Z","receivedAt":"2022-12-14T15:35:36Z","isPatch":true,"sender":{"key":"ckelsch@jgrcpa.com","avatar":null},"body":"From: Seija Kijin <doremylover123@gmail.com>\n\nMany of these casts remain,\neven though the target variable is the type it is being casted to.\nWe can safely remove said casts.\n\nSigned-off-by: Seija Kijin <doremylover123@gmail.com>\n---\n    git: remove unneeded casts\n    \n    Many of these casts remain, even though the target variable is the type\n    it is being casted to. We can safely remove said casts.\n    \n    Signed-off-by: Seija Kijin doremylover123@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1396%2FAtariDreams%2Ffix-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1396/AtariDreams/fix-v1\nPull-Request: https://github.com/git/git/pull/1396\n\n builtin/credential-store.c    |  2 +-\n diff.c                        |  8 ++++----\n http.c                        |  4 ++--\n imap-send.c                   |  5 +++--\n merge-ort.c                   |  2 +-\n oidmap.h                      |  2 +-\n pack-revindex.c               |  3 ++-\n read-cache.c                  |  2 +-\n ref-filter.c                  |  2 +-\n strbuf.c                      |  2 +-\n t/helper/test-parse-options.c |  2 +-\n t/helper/test-path-utils.c    |  8 ++++++--\n t/helper/test-run-command.c   | 10 +++++-----\n t/helper/test-subprocess.c    |  2 +-\n thread-utils.c                |  2 +-\n xdiff/xprepare.c              |  2 +-\n xdiff/xutils.c                |  2 +-\n 17 files changed, 33 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin/credential-store.c b/builtin/credential-store.c\nindex 62a4f3c2653..93e521af395 100644\n--- a/builtin/credential-store.c\n+++ b/builtin/credential-store.c\n@@ -165,7 +165,7 @@ int cmd_credential_store(int argc, const char **argv, const char *prefix)\n \n \tumask(077);\n \n-\targc = parse_options(argc, (const char **)argv, prefix, options, usage, 0);\n+\targc = parse_options(argc, argv, prefix, options, usage, 0);\n \tif (argc != 1)\n \t\tusage_with_options(usage, options);\n \top = argv[0];\ndiff --git a/diff.c b/diff.c\nindex 1054a4b7329..2e58c9372b4 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1776,8 +1776,8 @@ static void emit_rewrite_diff(const char *name_a,\n \tecbdata.opt = o;\n \tif (ecbdata.ws_rule & WS_BLANK_AT_EOF) {\n \t\tmmfile_t mf1, mf2;\n-\t\tmf1.ptr = (char *)data_one;\n-\t\tmf2.ptr = (char *)data_two;\n+\t\tmf1.ptr = data_one;\n+\t\tmf2.ptr = data_two;\n \t\tmf1.size = size_one;\n \t\tmf2.size = size_two;\n \t\tcheck_blank_at_eof(&mf1, &mf2, &ecbdata);\n@@ -1809,9 +1809,9 @@ static void emit_rewrite_diff(const char *name_a,\n \tif (lc_b)\n \t\temit_rewrite_lines(&ecbdata, '+', data_two, size_two);\n \tif (textconv_one)\n-\t\tfree((char *)data_one);\n+\t\tfree(data_one);\n \tif (textconv_two)\n-\t\tfree((char *)data_two);\n+\t\tfree(data_two);\n }\n \n struct diff_words_buffer {\ndiff --git a/http.c b/http.c\nindex 8a5ba3f4776..32db5d76a7c 100644\n--- a/http.c\n+++ b/http.c\n@@ -2319,8 +2319,8 @@ static size_t fwrite_sha1_file(char *ptr, size_t eltsize, size_t nmemb,\n \t}\n \n \tdo {\n-\t\tssize_t retval = xwrite(freq->localfile,\n-\t\t\t\t\t(char *) ptr + posn, size - posn);\n+\t\tssize_t retval =\n+\t\t\txwrite(freq->localfile, ptr + posn, size - posn);\n \t\tif (retval < 0)\n \t\t\treturn posn / eltsize;\n \t\tposn += retval;\ndiff --git a/imap-send.c b/imap-send.c\nindex a50af56b827..e67dbfc5567 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -779,7 +779,7 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)\n \t\t\tif (cmdp->cb.data) {\n \t\t\t\tn = socket_write(&imap->buf.sock, cmdp->cb.data, cmdp->cb.dlen);\n \t\t\t\tFREE_AND_NULL(cmdp->cb.data);\n-\t\t\t\tif (n != (int)cmdp->cb.dlen)\n+\t\t\t\tif (n != cmdp->cb.dlen)\n \t\t\t\t\treturn RESP_BAD;\n \t\t\t} else if (cmdp->cb.cont) {\n \t\t\t\tif (cmdp->cb.cont(ctx, cmdp, cmd))\n@@ -1526,7 +1526,8 @@ int cmd_main(int argc, const char **argv)\n \tsetup_git_directory_gently(&nongit_ok);\n \tgit_config(git_imap_config, NULL);\n \n-\targc = parse_options(argc, (const char **)argv, \"\", imap_send_options, imap_send_usage, 0);\n+\targc = parse_options(argc, argv, \"\", imap_send_options, imap_send_usage,\n+\t\t\t     0);\n \n \tif (argc)\n \t\tusage_with_options(imap_send_usage, imap_send_options);\ndiff --git a/merge-ort.c b/merge-ort.c\nindex d1611ca400a..a2aefd609ad 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -2574,7 +2574,7 @@ static void apply_directory_rename_modifications(struct merge_options *opt,\n \n \t/* Find parent directories missing from opt->priv->paths */\n \tcur_path = mem_pool_strdup(&opt->priv->pool, new_path);\n-\tfree((char*)new_path);\n+\tfree(new_path);\n \tnew_path = (char *)cur_path;\n \n \twhile (1) {\ndiff --git a/oidmap.h b/oidmap.h\nindex c66a83ab1d6..9cefbba550d 100644\n--- a/oidmap.h\n+++ b/oidmap.h\n@@ -87,7 +87,7 @@ static inline void *oidmap_iter_first(struct oidmap *map,\n {\n \toidmap_iter_init(map, iter);\n \t/* TODO: this API could be reworked to do compile-time type checks */\n-\treturn (void *)oidmap_iter_next(iter);\n+\treturn oidmap_iter_next(iter);\n }\n \n #endif\ndiff --git a/pack-revindex.c b/pack-revindex.c\nindex 08dc1601679..fa897b54584 100644\n--- a/pack-revindex.c\n+++ b/pack-revindex.c\n@@ -428,7 +428,8 @@ static int midx_pack_order_cmp(const void *va, const void *vb)\n \tconst struct midx_pack_key *key = va;\n \tstruct multi_pack_index *midx = key->midx;\n \n-\tuint32_t versus = pack_pos_to_midx(midx, (uint32_t*)vb - (const uint32_t *)midx->revindex_data);\n+\tuint32_t versus =\n+\t\tpack_pos_to_midx(midx, (uint32_t *)vb - midx->revindex_data);\n \tuint32_t versus_pack = nth_midxed_pack_int_id(midx, versus);\n \toff_t versus_offset;\n \ndiff --git a/read-cache.c b/read-cache.c\nindex 1ff518b2a7f..f4c4cc63dc4 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -3022,7 +3022,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \n \t\t\toffset = hashfile_total(f);\n \t\t}\n-\t\tif (ce_write_entry(f, ce, previous_name, (struct ondisk_cache_entry *)&ondisk) < 0)\n+\t\tif (ce_write_entry(f, ce, previous_name, &ondisk) < 0)\n \t\t\terr = -1;\n \n \t\tif (err)\ndiff --git a/ref-filter.c b/ref-filter.c\nindex caf10ab23eb..e5993f7cc43 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -396,7 +396,7 @@ static int trailers_atom_parser(struct ref_format *format, struct used_atom *ato\n \t\t\t\tstrbuf_addf(err, _(\"expected %%(trailers:key=<value>)\"));\n \t\t\telse\n \t\t\t\tstrbuf_addf(err, _(\"unknown %%(trailers) argument: %s\"), invalid_arg);\n-\t\t\tfree((char *)invalid_arg);\n+\t\t\tfree(invalid_arg);\n \t\t\treturn -1;\n \t\t}\n \t}\ndiff --git a/strbuf.c b/strbuf.c\nindex 0890b1405c5..940f59473eb 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -493,7 +493,7 @@ void strbuf_add_percentencode(struct strbuf *dst, const char *src, int flags)\n \t\tif (ch <= 0x1F || ch >= 0x7F ||\n \t\t    (ch == '/' && (flags & STRBUF_ENCODE_SLASH)) ||\n \t\t    strchr(URL_UNSAFE_CHARS, ch))\n-\t\t\tstrbuf_addf(dst, \"%%%02X\", (unsigned char)ch);\n+\t\t\tstrbuf_addf(dst, \"%%%02X\", ch);\n \t\telse\n \t\t\tstrbuf_addch(dst, ch);\n \t}\ndiff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c\nindex 506835521a4..a788196adf5 100644\n--- a/t/helper/test-parse-options.c\n+++ b/t/helper/test-parse-options.c\n@@ -161,7 +161,7 @@ int cmd__parse_options(int argc, const char **argv)\n \n \ttrace2_cmd_name(\"_parse_\");\n \n-\targc = parse_options(argc, (const char **)argv, prefix, options, usage, 0);\n+\targc = parse_options(argc, argv, prefix, options, usage, 0);\n \n \tif (length_cb.called) {\n \t\tconst char *arg = length_cb.arg;\ndiff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c\nindex f69709d674f..eb7a204f3b7 100644\n--- a/t/helper/test-path-utils.c\n+++ b/t/helper/test-path-utils.c\n@@ -276,13 +276,17 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)\n \t\t\t\tfor (j = 0; j < nr; j++)\n \t\t\t\t\tverify_path(names[j], file_mode);\n \t\t\t\tend = getnanotime();\n-\t\t\t\tprintf(\"protect_ntfs = %d, protect_hfs = %d: %lfms\\n\", protect_ntfs, protect_hfs, (end-begin) / (double)1e6);\n+\t\t\t\tprintf(\"protect_ntfs = %d, protect_hfs = %d: %lfms\\n\",\n+\t\t\t\t       protect_ntfs, protect_hfs,\n+\t\t\t\t       (end - begin) / 1e6);\n \t\t\t\tcumul += end - begin;\n \t\t\t\tcumul2 += (end - begin) * (end - begin);\n \t\t\t}\n \t\t\tm[protect_ntfs][protect_hfs] = cumul / (double)repetitions;\n \t\t\tv[protect_ntfs][protect_hfs] = my_sqrt(cumul2 / (double)repetitions - m[protect_ntfs][protect_hfs] * m[protect_ntfs][protect_hfs]);\n-\t\t\tprintf(\"mean: %lfms, stddev: %lfms\\n\", m[protect_ntfs][protect_hfs] / (double)1e6, v[protect_ntfs][protect_hfs] / (double)1e6);\n+\t\t\tprintf(\"mean: %lfms, stddev: %lfms\\n\",\n+\t\t\t       m[protect_ntfs][protect_hfs] / 1e6,\n+\t\t\t       v[protect_ntfs][protect_hfs] / 1e6);\n \t\t}\n \n \tfor (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)\ndiff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c\nindex 3ecb830f4a8..eadde7c47b1 100644\n--- a/t/helper/test-run-command.c\n+++ b/t/helper/test-run-command.c\n@@ -312,8 +312,8 @@ static int quote_stress_test(int argc, const char **argv)\n \t\tif (ret) {\n \t\t\tfprintf(stderr, \"Trial #%d failed. Arguments:\\n\", i);\n \t\t\tfor (j = 0; j < arg_count; j++)\n-\t\t\t\tfprintf(stderr, \"arg #%d: '%s'\\n\",\n-\t\t\t\t\t(int)j, args.v[j + arg_offset]);\n+\t\t\t\tfprintf(stderr, \"arg #%d: '%s'\\n\", j,\n+\t\t\t\t\targs.v[j + arg_offset]);\n \n \t\t\tstrbuf_release(&out);\n \t\t\tstrvec_clear(&args);\n@@ -322,7 +322,7 @@ static int quote_stress_test(int argc, const char **argv)\n \t\t}\n \n \t\tif (i && (i % 100) == 0)\n-\t\t\tfprintf(stderr, \"Trials completed: %d\\n\", (int)i);\n+\t\t\tfprintf(stderr, \"Trials completed: %d\\n\", i);\n \t}\n \n \tstrbuf_release(&out);\n@@ -418,7 +418,7 @@ int cmd__run_command(int argc, const char **argv)\n \t\tret = 1;\n \t\tgoto cleanup;\n \t}\n-\tstrvec_pushv(&proc.args, (const char **)argv + 2);\n+\tstrvec_pushv(&proc.args, argv + 2);\n \n \tif (!strcmp(argv[1], \"start-command-ENOENT\")) {\n \t\tif (start_command(&proc) < 0 && errno == ENOENT) {\n@@ -441,7 +441,7 @@ int cmd__run_command(int argc, const char **argv)\n \n \tjobs = atoi(argv[2]);\n \tstrvec_clear(&proc.args);\n-\tstrvec_pushv(&proc.args, (const char **)argv + 3);\n+\tstrvec_pushv(&proc.args, argv + 3);\n \n \tif (!strcmp(argv[1], \"run-command-parallel\")) {\n \t\topts.get_next_task = parallel_next;\ndiff --git a/t/helper/test-subprocess.c b/t/helper/test-subprocess.c\nindex ff22f2fa2c5..32a5b339eae 100644\n--- a/t/helper/test-subprocess.c\n+++ b/t/helper/test-subprocess.c\n@@ -15,6 +15,6 @@ int cmd__subprocess(int argc, const char **argv)\n \t\targv++;\n \t}\n \tcp.git_cmd = 1;\n-\tstrvec_pushv(&cp.args, (const char **)argv + 1);\n+\tstrvec_pushv(&cp.args, argv + 1);\n \treturn run_command(&cp);\n }\ndiff --git a/thread-utils.c b/thread-utils.c\nindex 53298456913..6d3de821473 100644\n--- a/thread-utils.c\n+++ b/thread-utils.c\n@@ -57,7 +57,7 @@ int online_cpus(void)\n #endif /* defined(HAVE_BSD_SYSCTL) && defined(HW_NCPU) */\n \n #ifdef _SC_NPROCESSORS_ONLN\n-\tif ((ncpus = (long)sysconf(_SC_NPROCESSORS_ONLN)) > 0)\n+\tif ((ncpus = sysconf(_SC_NPROCESSORS_ONLN)) > 0)\n \t\treturn (int)ncpus;\n #endif\n \ndiff --git a/xdiff/xprepare.c b/xdiff/xprepare.c\nindex c84549f6c50..04fa4e5a01d 100644\n--- a/xdiff/xprepare.c\n+++ b/xdiff/xprepare.c\n@@ -188,7 +188,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_\n \t\t\tif (!(crec = xdl_cha_alloc(&xdf->rcha)))\n \t\t\t\tgoto abort;\n \t\t\tcrec->ptr = prev;\n-\t\t\tcrec->size = (long) (cur - prev);\n+\t\t\tcrec->size = (cur - prev);\n \t\t\tcrec->ha = hav;\n \t\t\trecs[nrec++] = crec;\n \t\t\tif (xdl_classify_record(pass, cf, rhash, hbits, crec) < 0)\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 9e36f24875d..853f2260a1d 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -130,7 +130,7 @@ long xdl_guess_lines(mmfile_t *mf, long sample) {\n \t\t\telse\n \t\t\t\tcur++;\n \t\t}\n-\t\ttsize += (long) (cur - data);\n+\t\ttsize += (cur - data);\n \t}\n \n \tif (nl && tsize)\n\nbase-commit: 57e2c6ebbe7108b35ba30184dcbcb6c34c929ad8\n-- \ngitgitgadget\n"},{"id":"469044","messageId":"Y5o9NNVx7dFLhIMd@coredump.intra.peff.net","threadId":"58949","inReplyTo":"pull.1396.git.git.1671032126602.gitgitgadget@gmail.com","subject":"Re: [PATCH] git: remove unneeded casts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-14T21:16:36Z","receivedAt":"2022-12-14T21:16:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 14, 2022 at 03:35:26PM +0000, Rose via GitGitGadget wrote:\n\n> From: Seija Kijin <doremylover123@gmail.com>\n> \n> Many of these casts remain,\n> even though the target variable is the type it is being casted to.\n> We can safely remove said casts.\n\nFunny line-breaking. Re-wrapping with vim yields:\n\n  Many of these casts remain, even though the target variable\n  is the type it is being cast to.  We can safely remove\n  said casts.\n\nI also fixed a minor grammatical issue with \"casted\".\n\nI'm curious how you found all of these. With coccinelle or\nsimilar? If there's an analysis tool that can find them,\nthat would make review much simpler if we can trust that it\ndid the leg-work of checking the types.\n\nEither way, I think it's still useful to see how we ended up\nin this state for each case (so we know that cleaning up the\ncast is the right thing, and it is not a symptom of some\nother bug).\n\n> diff --git a/builtin/credential-store.c b/builtin/credential-store.c\n> index 62a4f3c2653..93e521af395 100644\n> --- a/builtin/credential-store.c\n> +++ b/builtin/credential-store.c\n> @@ -165,7 +165,7 @@ int cmd_credential_store(int argc, const char **argv, const char *prefix)\n>  \n>  \tumask(077);\n>  \n> -\targc = parse_options(argc, (const char **)argv, prefix, options, usage, 0);\n> +\targc = parse_options(argc, argv, prefix, options, usage, 0);\n>  \tif (argc != 1)\n>  \t\tusage_with_options(usage, options);\n>  \top = argv[0];\n\nOur argv used to be \"char **\" due to 84d32bf767 (sparse: Fix\nmingw_main() argument number/type errors, 2013-04-27), but\nlater regained its const when it became cmd_main() in\n3f2e2297b9 (add an extra level of indirection to main(),\n2016-07-01). Makes sense.\n\n> diff --git a/diff.c b/diff.c\n> index 1054a4b7329..2e58c9372b4 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1776,8 +1776,8 @@ static void emit_rewrite_diff(const char *name_a,\n>  \tecbdata.opt = o;\n>  \tif (ecbdata.ws_rule & WS_BLANK_AT_EOF) {\n>  \t\tmmfile_t mf1, mf2;\n> -\t\tmf1.ptr = (char *)data_one;\n> -\t\tmf2.ptr = (char *)data_two;\n> +\t\tmf1.ptr = data_one;\n> +\t\tmf2.ptr = data_two;\n>  \t\tmf1.size = size_one;\n>  \t\tmf2.size = size_two;\n>  \t\tcheck_blank_at_eof(&mf1, &mf2, &ecbdata);\n> @@ -1809,9 +1809,9 @@ static void emit_rewrite_diff(const char *name_a,\n>  \tif (lc_b)\n>  \t\temit_rewrite_lines(&ecbdata, '+', data_two, size_two);\n>  \tif (textconv_one)\n> -\t\tfree((char *)data_one);\n> +\t\tfree(data_one);\n>  \tif (textconv_two)\n> -\t\tfree((char *)data_two);\n> +\t\tfree(data_two);\n>  }\n\nThese variables lost their const in 840383b2c2 (textconv:\nrefactor calls to run_textconv, 2010-04-01). OK, makes\nsense.\n\n> diff --git a/http.c b/http.c\n> index 8a5ba3f4776..32db5d76a7c 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -2319,8 +2319,8 @@ static size_t fwrite_sha1_file(char *ptr, size_t eltsize, size_t nmemb,\n>  \t}\n>  \n>  \tdo {\n> -\t\tssize_t retval = xwrite(freq->localfile,\n> -\t\t\t\t\t(char *) ptr + posn, size - posn);\n> +\t\tssize_t retval =\n> +\t\t\txwrite(freq->localfile, ptr + posn, size - posn);\n>  \t\tif (retval < 0)\n>  \t\t\treturn posn / eltsize;\n>  \t\tposn += retval;\n\nThis one went from a void pointer to a char pointer in\na04ff3ec32 (http: make curl callbacks match contracts from\ncurl header, 2011-05-03). Good.\n\n> diff --git a/imap-send.c b/imap-send.c\n> index a50af56b827..e67dbfc5567 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -779,7 +779,7 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)\n>  \t\t\tif (cmdp->cb.data) {\n>  \t\t\t\tn = socket_write(&imap->buf.sock, cmdp->cb.data, cmdp->cb.dlen);\n>  \t\t\t\tFREE_AND_NULL(cmdp->cb.data);\n> -\t\t\t\tif (n != (int)cmdp->cb.dlen)\n> +\t\t\t\tif (n != cmdp->cb.dlen)\n>  \t\t\t\t\treturn RESP_BAD;\n>  \t\t\t} else if (cmdp->cb.cont) {\n>  \t\t\t\tif (cmdp->cb.cont(ctx, cmdp, cmd))\n\nThis one dates back to f2561fda36 (Add git-imap-send,\nderived from isync 1.0.1., 2006-03-10), so no clue how it\ncame about. It's clearly pointless, though, as \"dlen\" is\nalready an int.\n\n> @@ -1526,7 +1526,8 @@ int cmd_main(int argc, const char **argv)\n>  \tsetup_git_directory_gently(&nongit_ok);\n>  \tgit_config(git_imap_config, NULL);\n>  \n> -\targc = parse_options(argc, (const char **)argv, \"\", imap_send_options, imap_send_usage, 0);\n> +\targc = parse_options(argc, argv, \"\", imap_send_options, imap_send_usage,\n> +\t\t\t     0);\n\nThis is another one that picked up a const when switching to\ncmd_main().\n\n> diff --git a/merge-ort.c b/merge-ort.c\n> index d1611ca400a..a2aefd609ad 100644\n> --- a/merge-ort.c\n> +++ b/merge-ort.c\n> @@ -2574,7 +2574,7 @@ static void apply_directory_rename_modifications(struct merge_options *opt,\n>  \n>  \t/* Find parent directories missing from opt->priv->paths */\n>  \tcur_path = mem_pool_strdup(&opt->priv->pool, new_path);\n> -\tfree((char*)new_path);\n> +\tfree(new_path);\n>  \tnew_path = (char *)cur_path;\n\nThis one was always a \"char *\", going back to fa5e06d690\n(merge-ort: modify collect_renames() for directory rename\nhandling, 2021-01-19).\n\nThere's another variable in there, parent_name, which\nprobably _should_ be non-const, since we allocate and free\nit. That's out of scope for your patch, though.\n\n> diff --git a/oidmap.h b/oidmap.h\n> index c66a83ab1d6..9cefbba550d 100644\n> --- a/oidmap.h\n> +++ b/oidmap.h\n> @@ -87,7 +87,7 @@ static inline void *oidmap_iter_first(struct oidmap *map,\n>  {\n>  \toidmap_iter_init(map, iter);\n>  \t/* TODO: this API could be reworked to do compile-time type checks */\n> -\treturn (void *)oidmap_iter_next(iter);\n> +\treturn oidmap_iter_next(iter);\n>  }\n\nThis comes from 87571c3f71 (hashmap: use *_entry APIs for\niteration, 2019-10-06), whose only change to this line is\nadding the cast. Maybe the intent was to flag the area to be\nhandled later according to the comment? I'd think the\ncomment itself suffices, there. But if not, then we're\nundoing it.\n\nThere's an identical case (from the same commit) a few lines\nabove. If we are changing this one, we should do both.\n\nI'll stop here for now. It's a fair bit of leg-work digging\nthese up (though again, I do think there's value in\nunderstanding why the cast was there, even if we know it\nisn't _currently_ doing anything).\n\n-Peff\n"},{"id":"469058","messageId":"xmqqh6xxv946.fsf@gitster.g","threadId":"58949","inReplyTo":"Y5o9NNVx7dFLhIMd@coredump.intra.peff.net","subject":"Re: [PATCH] git: remove unneeded casts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-14T23:49:45Z","receivedAt":"2022-12-14T23:49:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I'll stop here for now. It's a fair bit of leg-work digging\n> these up (though again, I do think there's value in\n> understanding why the cast was there, even if we know it\n> isn't _currently_ doing anything).\n\nI agree with the value of understanding why each of these casts has\nbecome unnecessary, and thanks for demonstrating how a rerolled\nversion should justify its changes with its findings behind each of\nthe unnecessary casts.\n\nWhat do you recommend the next round should look like?  Multi patch\nseries, each of which removes one cast with its proposed log message\nexplains how it has become unneeded?  A single patch with a gigantic\nproposed log message that lists the findings for each and all casts\nthat are removed?  Somewhere in between, perhaps split along the\nfile boundary, or grouped by the event that made them unneeded\n(e.g. \"cmd_main() used to take non-const but when we made it to take\nconst, all of these casts we remove in this patch became unneeded\")?\n\nThanks.\n\n\n\n"},{"id":"469115","messageId":"5352a684-8d47-ea1d-ee87-92e4722d47ce@dunelm.org.uk","threadId":"58949","inReplyTo":"pull.1396.git.git.1671032126602.gitgitgadget@gmail.com","subject":"Re: [PATCH] git: remove unneeded casts","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-12-15T10:18:29Z","receivedAt":"2022-12-15T10:18:36Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 14/12/2022 15:35, Rose via GitGitGadget wrote:\n> From: Seija Kijin <doremylover123@gmail.com>\n> \n> Many of these casts remain,\n> even though the target variable is the type it is being casted to.\n> We can safely remove said casts.\n\nLike Peff I curious how you found these. I also agree that it would be \nhelpful to understand the history so we can be sure there the cast is \nnot a sympton of a bug - Peff has given you a great start on that. A lot \nof the changes look like useful cleanups but there are a couple that \ncaught my eye as being wrong or undesirable which I've commented on below.\n\n> diff --git a/strbuf.c b/strbuf.c\n> index 0890b1405c5..940f59473eb 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -493,7 +493,7 @@ void strbuf_add_percentencode(struct strbuf *dst, const char *src, int flags)\n>   \t\tif (ch <= 0x1F || ch >= 0x7F ||\n>   \t\t    (ch == '/' && (flags & STRBUF_ENCODE_SLASH)) ||\n>   \t\t    strchr(URL_UNSAFE_CHARS, ch))\n> -\t\t\tstrbuf_addf(dst, \"%%%02X\", (unsigned char)ch);\n> +\t\t\tstrbuf_addf(dst, \"%%%02X\", ch);\n\nThis one is wrong, the cast is required if char is signed as in that \ncase it will be converted to a signed integer (because printf() takes a \nvariable number of arguments) and then %X treats that integer as \nunsigned. That means that if the high bit is set we'll now print a bunch \nof leading F's. To see this compile and run\n\n#include <stdio.h>\n\nint main(void)\n{\n         printf(\"%02X %02X\\n\", (signed char)0x80, (unsigned char)0x80);\n         return 0;\n}\n\nwhich prints\n\nFFFFFF80 80\n\n> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c\n> index c84549f6c50..04fa4e5a01d 100644\n> --- a/xdiff/xprepare.c\n> +++ b/xdiff/xprepare.c\n> @@ -188,7 +188,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_\n>   \t\t\tif (!(crec = xdl_cha_alloc(&xdf->rcha)))\n>   \t\t\t\tgoto abort;\n>   \t\t\tcrec->ptr = prev;\n> -\t\t\tcrec->size = (long) (cur - prev);\n> +\t\t\tcrec->size = (cur - prev);\n\nI'm not sure if we want to remove this cast. cur and prev are pointers \nso (cur - prev) has type ptrdiff_t which maybe wider than long. We can \ndebate whether we should be using a cast to hide any compiler warning \nhere but the cast is not redundant in the same way as others in this patch.\n\n>   \t\t\tcrec->ha = hav;\n>   \t\t\trecs[nrec++] = crec;\n>   \t\t\tif (xdl_classify_record(pass, cf, rhash, hbits, crec) < 0)\n> diff --git a/xdiff/xutils.c b/xdiff/xutils.c\n> index 9e36f24875d..853f2260a1d 100644\n> --- a/xdiff/xutils.c\n> +++ b/xdiff/xutils.c\n> @@ -130,7 +130,7 @@ long xdl_guess_lines(mmfile_t *mf, long sample) {\n>   \t\t\telse\n>   \t\t\t\tcur++;\n>   \t\t}\n> -\t\ttsize += (long) (cur - data);\n> +\t\ttsize += (cur - data);\n\nThis is in the same class as the one above. (Also if we do end up \nremoving the cast we should remove the parentheses as well)\n\nBest Wishes\n\nPhillip\n"},{"id":"469116","messageId":"221215.86edt12bte.gmgdl@evledraar.gmail.com","threadId":"58949","inReplyTo":"Y5o9NNVx7dFLhIMd@coredump.intra.peff.net","subject":"Re: [PATCH] git: remove unneeded casts","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-15T10:07:24Z","receivedAt":"2022-12-15T10:36:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Dec 14 2022, Jeff King wrote:\n\n> On Wed, Dec 14, 2022 at 03:35:26PM +0000, Rose via GitGitGadget wrote:\n> [...]\n>> diff --git a/oidmap.h b/oidmap.h\n>> index c66a83ab1d6..9cefbba550d 100644\n>> --- a/oidmap.h\n>> +++ b/oidmap.h\n>> @@ -87,7 +87,7 @@ static inline void *oidmap_iter_first(struct oidmap *map,\n>>  {\n>>  \toidmap_iter_init(map, iter);\n>>  \t/* TODO: this API could be reworked to do compile-time type checks */\n>> -\treturn (void *)oidmap_iter_next(iter);\n>> +\treturn oidmap_iter_next(iter);\n>>  }\n>\n> This comes from 87571c3f71 (hashmap: use *_entry APIs for\n> iteration, 2019-10-06), whose only change to this line is\n> adding the cast. Maybe the intent was to flag the area to be\n> handled later according to the comment? I'd think the\n> comment itself suffices, there. But if not, then we're\n> undoing it.\n>\n> There's an identical case (from the same commit) a few lines\n> above. If we are changing this one, we should do both.\n>\n> I'll stop here for now. It's a fair bit of leg-work digging\n> these up (though again, I do think there's value in\n> understanding why the cast was there, even if we know it\n> isn't _currently_ doing anything).\n\nThanks for doing all that leg-work. Whan I saw this patch earlier I was\ngoing to point out the same.\n\nI.e. we really should know why these are there in the first place, or\nalterantively convince ourselves that e.g a wide-ranging coccinelle rule\nwe're applying is safe.\n\nI wondered if we could find potential candidates with a brute-force\napproach, this is very much a hack, but perhaps for something for Rose\nto chew on (either for this topic, or as a follow-up):\n\n\t# Could adjust this regex to catch more types...\n\tperl -pi -e 's/\\((void|const char|char)\\s*\\*\\)\\s*//g' $(git ls-files '*.[ch]')\n        # Commit a hunk at a time...\n\twhile true; do echo y | git add -p && git commit -m\"bump\"; done\n\nThis gives you around ~500 commits. It's also inaccurate, e.g. it ruined\nsome function-prototypes, edited comments etc. But let's look past that\nfor now.\n\nThen, as even further brute-forcing, let's get rid of those commits\ngenerated above where the compiler doesn't like it:\n\n\tgit rebase -i -x 'make objects || git reset --hard HEAD^1 && make objects'\n\nNow, I'd never suggest that we actually apply the result of this\nmonkeypatching.\n\nEven if the compiler could spot logic errors in this with 100%\naccuratecy the above would still be broken, as it won't compile all\npossible codepaths (e.g. in compat/*, which I should have left out to\nbegin with).\n\nBut it *is* useful to see common patterns that might serve as potential\ncandidates, and it even spots a case where e.g. the \"mf.ptr\" still has a\ncast in diff.c, so that seems to be missed from the upthread patch.\n\nRose: It's really much more useful to edit contrib/coccinelle/*.cocci to\nadd a rule saying \"whenever we have a type X being assigned to, cast to\nY, that Y cast can be removed\". The coccinelle engine is much better at\nspotting those than human eyes, or grep/search & replace.\n\nThose disclaimers aside, the above yields ~170 commits on top of the\nabove.\n\nExcluding compat/ the diff is the below. Some of it's broken, but some\nof it suggests things that are worth picking up, e.g. the return value\nof xmalloc() being cast (a C++-ism), \"(char *)NULL\" (do we ever need to\ncast NULL?)\n\n bisect.c                     |  2 +-\n block-sha1/sha1.c            |  6 +++---\n builtin/bisect.c             | 14 +++++++-------\n builtin/help.c               | 10 +++++-----\n builtin/receive-pack.c       |  2 +-\n builtin/replace.c            |  2 +-\n builtin/tag.c                |  2 +-\n bundle-uri.c                 |  2 +-\n connect.c                    |  2 +-\n diff.c                       |  6 +++---\n fsck.c                       |  2 +-\n git-compat-util.h            | 16 ++++++++--------\n grep.c                       |  2 +-\n hashmap.c                    |  2 +-\n http.c                       |  2 +-\n imap-send.c                  |  4 ++--\n khash.h                      |  4 ++--\n line-range.c                 |  4 ++--\n list.h                       |  2 +-\n notes.c                      |  2 +-\n object-name.c                |  2 +-\n oidmap.h                     |  2 +-\n pack-bitmap.c                |  2 +-\n pack-revindex.c              |  4 ++--\n read-cache.c                 |  6 +++---\n reftable/writer.c            |  4 ++--\n rerere.c                     |  6 +++---\n resolve-undo.c               |  2 +-\n revision.c                   |  2 +-\n sha1dc/sha1.c                |  2 +-\n t/helper/test-rot13-filter.c |  4 ++--\n t/helper/test-run-command.c  |  4 ++--\n t/helper/test-simple-ipc.c   |  4 ++--\n t/helper/test-string-list.c  |  2 +-\n thread-utils.c               |  2 +-\n thread-utils.h               |  4 ++--\n xdiff/xutils.c               |  2 +-\n 37 files changed, 71 insertions(+), 71 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex ec7487e6836..440098991c3 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1179 +1179 @@ int bisect_clean_state(void)\n-\tfor_each_ref_in(\"refs/bisect\", mark_for_removal, (void *) &refs_for_removal);\n+\tfor_each_ref_in(\"refs/bisect\", mark_for_removal, &refs_for_removal);\ndiff --git a/block-sha1/sha1.c b/block-sha1/sha1.c\nindex 80cebd27564..9759febb120 100644\n--- a/block-sha1/sha1.c\n+++ b/block-sha1/sha1.c\n@@ -196 +196 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, size_t len)\n-\t\tmemcpy(lenW + (char *)ctx->W, data, left);\n+\t\tmemcpy(lenW + ctx->W, data, left);\n@@ -199 +199 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, size_t len)\n-\t\tdata = ((const char *)data + left);\n+\t\tdata = (data + left);\n@@ -206 +206 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, size_t len)\n-\t\tdata = ((const char *)data + 64);\n+\t\tdata = (data + 64);\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex cc9483e8515..50e5275efaa 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -79 +79 @@ static void set_terms(struct bisect_terms *terms, const char *bad,\n-\tfree((void *)terms->term_good);\n+\tfree(terms->term_good);\n@@ -81 +81 @@ static void set_terms(struct bisect_terms *terms, const char *bad,\n-\tfree((void *)terms->term_bad);\n+\tfree(terms->term_bad);\n@@ -419 +419 @@ static void bisect_status(struct bisect_state *state,\n-\t\t\t     (void *) &state->nr_good);\n+\t\t\t     &state->nr_good);\n@@ -723 +723 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, const char **a\n-\t\t\tfree((void *) terms->term_good);\n+\t\t\tfree(terms->term_good);\n@@ -728 +728 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, const char **a\n-\t\t\tfree((void *) terms->term_good);\n+\t\t\tfree(terms->term_good);\n@@ -736 +736 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, const char **a\n-\t\t\tfree((void *) terms->term_bad);\n+\t\t\tfree(terms->term_bad);\n@@ -741 +741 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, const char **a\n-\t\t\tfree((void *) terms->term_bad);\n+\t\t\tfree(terms->term_bad);\ndiff --git a/builtin/help.c b/builtin/help.c\nindex 53f2812dfb1..c42d4763994 100644\n--- a/builtin/help.c\n+++ b/builtin/help.c\n@@ -275 +275 @@ static void exec_woman_emacs(const char *path, const char *page)\n-\t\texeclp(path, \"emacsclient\", \"-e\", man_page.buf, (char *)NULL);\n+\t\texeclp(path, \"emacsclient\", \"-e\", man_page.buf, NULL);\n@@ -307 +307 @@ static void exec_man_man(const char *path, const char *page)\n-\texeclp(path, \"man\", page, (char *)NULL);\n+\texeclp(path, \"man\", page, NULL);\n@@ -315 +315 @@ static void exec_man_cmd(const char *cmd, const char *page)\n-\texecl(SHELL_PATH, SHELL_PATH, \"-c\", shell_cmd.buf, (char *)NULL);\n+\texecl(SHELL_PATH, SHELL_PATH, \"-c\", shell_cmd.buf, NULL);\n@@ -505 +505 @@ static void show_info_page(const char *page)\n-\texeclp(\"info\", \"info\", \"gitman\", page, (char *)NULL);\n+\texeclp(\"info\", \"info\", \"gitman\", page, NULL);\n@@ -534 +534 @@ static void open_html(const char *path)\n-\texecl_git_cmd(\"web--browse\", \"-c\", \"help.browser\", path, (char *)NULL);\n+\texecl_git_cmd(\"web--browse\", \"-c\", \"help.browser\", path, NULL);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex a90af303630..bc229f112e0 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1714 +1714 @@ static void check_aliased_updates(struct command *commands)\n-\t\titem->util = (void *)cmd;\n+\t\titem->util = cmd;\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex a29e911d309..3a3bbb49d6d 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -94 +94 @@ static int list_replace_refs(const char *pattern, const char *format)\n-\tfor_each_replace_ref(the_repository, show_reference, (void *)&data);\n+\tfor_each_replace_ref(the_repository, show_reference, &data);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex d428c45dc8d..8c0be18cb11 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -130 +130 @@ static int delete_tags(const char **argv)\n-\tresult = for_each_tag_name(argv, collect_tags, (void *)&refs_to_delete);\n+\tresult = for_each_tag_name(argv, collect_tags, &refs_to_delete);\ndiff --git a/bundle-uri.c b/bundle-uri.c\nindex 79a914f961b..3813935ec69 100644\n--- a/bundle-uri.c\n+++ b/bundle-uri.c\n@@ -21 +21 @@ static int compare_bundles(const void *hashmap_cmp_fn_data,\n-\treturn strcmp(e1->id, id ? (const char *)id : e2->id);\n+\treturn strcmp(e1->id, id ? id : e2->id);\ndiff --git a/connect.c b/connect.c\nindex 5ea53deda23..52bf52f2f91 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -219 +219 @@ static void annotate_refs_with_symref_info(struct ref *ref)\n-\t\tref->symref = xstrdup((char *)item->util);\n+\t\tref->symref = xstrdup(item->util);\ndiff --git a/diff.c b/diff.c\nindex 2e58c9372b4..6f0951350bf 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -581 +581 @@ static int fill_mmfile(struct repository *r, mmfile_t *mf,\n-\t\tmf->ptr = (char *)\"\"; /* does not matter */\n+\t\tmf->ptr = \"\"; /* does not matter */\n@@ -4058 +4058 @@ int diff_populate_filespec(struct repository *r,\n-\t\t\ts->data = (char *)\"\";\n+\t\t\ts->data = \"\";\n@@ -4201 +4201 @@ static void prep_temp_blob(struct index_state *istate,\n-\t\t\t(const char *)blob, (size_t)size, &buf, &meta)) {\n+\t\t\tblob, (size_t)size, &buf, &meta)) {\ndiff --git a/fsck.c b/fsck.c\nindex b3da1d68c0b..4183ca065c1 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -749 +749 @@ static int verify_headers(const void *data, unsigned long size,\n-\tconst char *buffer = (const char *)data;\n+\tconst char *buffer = data;\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex a76d0526f79..cf0e275621e 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -89 +89 @@ struct strbuf;\n- *\t\t ((char *)(foo)\t\t\t\t\t\t\\\n+ *\t\t ((foo)\t\t\t\t\t\t\\\n@@ -302 +302 @@ static inline int is_xplatform_dir_sep(int c)\n- * Size needs to match (or exceed) 'sizeof(void *)'.\n+ * Size needs to match (or exceed) 'sizeof'.\n@@ -355 +355 @@ static inline int setitimer(int which UNUSED,\n-char *gitbasename(char *);\n+char *gitbasename;\n@@ -357 +357 @@ char *gitbasename(char *);\n-char *gitdirname(char *);\n+char *gitdirname;\n@@ -868 +868 @@ int gitsetenv(const char *, const char *, int);\n-char *gitmkdtemp(char *);\n+char *gitmkdtemp;\n@@ -873 +873 @@ char *gitmkdtemp(char *);\n-int gitunsetenv(const char *);\n+int gitunsetenv;\n@@ -963 +963 @@ static inline char *gitstrchrnul(const char *s, int c)\n-\treturn (char *)s;\n+\treturn s;\n@@ -1573 +1573 @@ int uncompress2(Bytef *dest, uLongf *destLen, const Bytef *source,\n-\t((type *) ((char *)(ptr) - offsetof(type, member)))\n+\t((type *) ((ptr) - offsetof(type, member)))\ndiff --git a/grep.c b/grep.c\nindex 06eed694936..de7c28776fb 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -391 +391 @@ static void free_pcre2_pattern(struct grep_pat *p)\n-\tfree((void *)p->pcre2_tables);\n+\tfree(p->pcre2_tables);\ndiff --git a/hashmap.c b/hashmap.c\nindex cf5fea87eb0..4a6a0d6b14a 100644\n--- a/hashmap.c\n+++ b/hashmap.c\n@@ -188 +188 @@ static void free_individual_entries(struct hashmap *map, ssize_t entry_offset)\n-\t\tfree((char *)e - entry_offset);\n+\t\tfree(e - entry_offset);\ndiff --git a/http.c b/http.c\nindex 32db5d76a7c..3a8ece892ed 100644\n--- a/http.c\n+++ b/http.c\n@@ -550 +550 @@ static int sockopt_callback(void *client, curl_socket_t fd, curlsocktype type)\n-\trc = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void *)&ka, len);\n+\trc = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &ka, len);\ndiff --git a/imap-send.c b/imap-send.c\nindex e67dbfc5567..a73471300d2 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -777 +777 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)\n-\t\t\tcmdp = (struct imap_cmd *)((char *)imap->in_progress_append -\n+\t\t\tcmdp = (struct imap_cmd *)(imap->in_progress_append -\n@@ -903 +903 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)\n-\treturn (char *)response_64;\n+\treturn response_64;\ndiff --git a/khash.h b/khash.h\nindex cb79bf88567..95ac4f4d9ab 100644\n--- a/khash.h\n+++ b/khash.h\n@@ -88,2 +88,2 @@ static const double __ac_HASH_UPPER = 0.77;\n-\t\tfree((void *)h->keys);\t\t\t\t\t\t\t\t\t\t\t\\\n-\t\tfree((void *)h->vals);\t\t\t\t\t\t\t\t\t\t\t\\\n+\t\tfree(h->keys);\t\t\t\t\t\t\t\t\t\t\t\\\n+\t\tfree(h->vals);\t\t\t\t\t\t\t\t\t\t\t\\\ndiff --git a/line-range.c b/line-range.c\nindex 955a8a95355..8ab64d95e7b 100644\n--- a/line-range.c\n+++ b/line-range.c\n@@ -160 +160 @@ static const char *find_funcname_matching_regexp(xdemitconf_t *xecfg, const char\n-\t\tif (match_funcname(xecfg, (char*) bol, (char*) eol))\n+\t\tif (match_funcname(xecfg, bol, eol))\n@@ -216 +216 @@ static const char *parse_range_funcname(\n-\tp = find_funcname_matching_regexp(xecfg, (char*) start, &regexp);\n+\tp = find_funcname_matching_regexp(xecfg, start, &regexp);\ndiff --git a/list.h b/list.h\nindex 362a4cd7f5f..bab41d6aa64 100644\n--- a/list.h\n+++ b/list.h\n@@ -122 +122 @@ static inline void list_splice(struct list_head *add, struct list_head *head)\n-\t((type *) ((char *) (ptr) - offsetof(type, member)))\n+\t((type *) ((ptr) - offsetof(type, member)))\ndiff --git a/notes.c b/notes.c\nindex f2805d51bb1..ffe2f030a56 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -830 +830 @@ int combine_notes_concatenate(struct object_id *cur_oid,\n-\tbuf = (char *) xmalloc(buf_len);\n+\tbuf = xmalloc(buf_len);\ndiff --git a/object-name.c b/object-name.c\nindex 2dd1a0f56e1..9908d65af17 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -808 +808 @@ int repo_find_unique_abbrev_r(struct repository *r, char *hex,\n-\tds.cb_data = (void *)&mad;\n+\tds.cb_data = &mad;\ndiff --git a/oidmap.h b/oidmap.h\nindex 9cefbba550d..c72e9c9c40e 100644\n--- a/oidmap.h\n+++ b/oidmap.h\n@@ -82 +82 @@ static inline void *oidmap_iter_next(struct oidmap_iter *iter)\n-\treturn (void *)hashmap_iter_next(&iter->h_iter);\n+\treturn hashmap_iter_next(&iter->h_iter);\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex d2a42abf28c..3a3115ad105 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -202 +202 @@ static int load_bitmap_header(struct bitmap_index *index)\n-\t\t\t\tindex->table_lookup = (void *)(index_end - table_size);\n+\t\t\t\tindex->table_lookup = (index_end - table_size);\ndiff --git a/pack-revindex.c b/pack-revindex.c\nindex fa897b54584..ffe034957fc 100644\n--- a/pack-revindex.c\n+++ b/pack-revindex.c\n@@ -278 +278 @@ static int load_pack_revindex_from_disk(struct packed_git *p)\n-\tp->revindex_data = (const uint32_t *)((const char *)p->revindex_map + RIDX_HEADER_SIZE);\n+\tp->revindex_data = (const uint32_t *)(p->revindex_map + RIDX_HEADER_SIZE);\n@@ -333 +333 @@ int load_midx_revindex(struct multi_pack_index *m)\n-\tm->revindex_data = (const uint32_t *)((const char *)m->revindex_map + RIDX_HEADER_SIZE);\n+\tm->revindex_data = (const uint32_t *)(m->revindex_map + RIDX_HEADER_SIZE);\ndiff --git a/read-cache.c b/read-cache.c\nindex f4c4cc63dc4..0f1a18ecf02 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1920 +1920 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n-\t\tname = (const char *)(flagsp + 2 * sizeof(uint16_t));\n+\t\tname = (flagsp + 2 * sizeof(uint16_t));\n@@ -1923 +1923 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n-\t\tname = (const char *)(flagsp + sizeof(uint16_t));\n+\t\tname = (flagsp + sizeof(uint16_t));\n@@ -1980 +1980 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,\n-\t\t*ent_size = (name - ((char *)ondisk)) + len + 1 - copy_len;\n+\t\t*ent_size = (name - (ondisk)) + len + 1 - copy_len;\ndiff --git a/reftable/writer.c b/reftable/writer.c\nindex 2e322a5683d..ee015525536 100644\n--- a/reftable/writer.c\n+++ b/reftable/writer.c\n@@ -193 +193 @@ static void writer_index_hash(struct reftable_writer *w, struct strbuf *hash)\n-\t\ttree_search((void *)key, &w->obj_index_tree,\n+\t\ttree_search(key, &w->obj_index_tree,\n@@ -280 +280 @@ int reftable_writer_add_ref(struct reftable_writer *w,\n-\t\tstrbuf_add(&h, (char *)reftable_ref_record_val1(ref),\n+\t\tstrbuf_add(&h, reftable_ref_record_val1(ref),\ndiff --git a/rerere.c b/rerere.c\nindex 876ab435da9..9b59fa7940b 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -556 +556 @@ static int find_conflict(struct repository *r, struct string_list *conflict)\n-\t\t\tstring_list_insert(conflict, (const char *)e->name);\n+\t\t\tstring_list_insert(conflict, e->name);\n@@ -590 +590 @@ int rerere_remaining(struct repository *r, struct string_list *merge_rr)\n-\t\t\tstring_list_insert(merge_rr, (const char *)e->name);\n+\t\t\tstring_list_insert(merge_rr, e->name);\n@@ -593 +593 @@ int rerere_remaining(struct repository *r, struct string_list *merge_rr)\n-\t\t\tit = string_list_lookup(merge_rr, (const char *)e->name);\n+\t\t\tit = string_list_lookup(merge_rr, e->name);\ndiff --git a/resolve-undo.c b/resolve-undo.c\nindex e81096e2d45..176186d7e9d 100644\n--- a/resolve-undo.c\n+++ b/resolve-undo.c\n@@ -81 +81 @@ struct string_list *resolve_undo_read(const char *data, unsigned long size)\n-\t\t\tlen = (endptr + 1) - (char*)data;\n+\t\t\tlen = (endptr + 1) - data;\ndiff --git a/revision.c b/revision.c\nindex c86e76e4716..93ef97e7e76 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -3909 +3909 @@ static int commit_match(struct commit *commit, struct rev_info *opt)\n-\t\t\t\t     (char *)message, strlen(message));\n+\t\t\t\t     message, strlen(message));\ndiff --git a/sha1dc/sha1.c b/sha1dc/sha1.c\nindex dede2cbddf9..901aff46cfb 100644\n--- a/sha1dc/sha1.c\n+++ b/sha1dc/sha1.c\n@@ -1873 +1873 @@ int SHA1DCFinal(unsigned char output[20], SHA1_CTX *ctx)\n-\tSHA1DCUpdate(ctx, (const char*)(sha1_padding), padn);\n+\tSHA1DCUpdate(ctx, (sha1_padding), padn);\ndiff --git a/t/helper/test-rot13-filter.c b/t/helper/test-rot13-filter.c\nindex f8d564c622a..43ae901fd2c 100644\n--- a/t/helper/test-rot13-filter.c\n+++ b/t/helper/test-rot13-filter.c\n@@ -67,2 +67,2 @@ static char *get_value(char *buf, const char *key)\n-\t    !skip_prefix((const char *)buf, key, (const char **)&buf) ||\n-\t    !skip_prefix((const char *)buf, \"=\", (const char **)&buf) ||\n+\t    !skip_prefix(buf, key, (const char **)&buf) ||\n+\t    !skip_prefix(buf, \"=\", (const char **)&buf) ||\ndiff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c\nindex eadde7c47b1..0d93ee1bcd7 100644\n--- a/t/helper/test-run-command.c\n+++ b/t/helper/test-run-command.c\n@@ -110 +110 @@ static int test_finished(int result, struct strbuf *err, void *cb,\n-\tconst char *name = (const char *)task_cb;\n+\tconst char *name = task_cb;\n@@ -123 +123 @@ static int test_failed(struct strbuf *out, void *cb, void *task_cb)\n-\tconst char *name = (const char *)task_cb;\n+\tconst char *name = task_cb;\ndiff --git a/t/helper/test-simple-ipc.c b/t/helper/test-simple-ipc.c\nindex 28365ff85b6..7830353e08d 100644\n--- a/t/helper/test-simple-ipc.c\n+++ b/t/helper/test-simple-ipc.c\n@@ -181 +181 @@ static int test_app_cb(void *application_data,\n-\tif (application_data != (void*)&my_app_data)\n+\tif (application_data != &my_app_data)\n@@ -269 +269 @@ static int daemon__run_server(void)\n-\tret = ipc_server_run(cl_args.path, &opts, test_app_cb, (void*)&my_app_data);\n+\tret = ipc_server_run(cl_args.path, &opts, test_app_cb, &my_app_data);\ndiff --git a/t/helper/test-string-list.c b/t/helper/test-string-list.c\nindex 2123dda85bf..43bea3ba6b3 100644\n--- a/t/helper/test-string-list.c\n+++ b/t/helper/test-string-list.c\n@@ -41 +41 @@ static int prefix_cb(struct string_list_item *item, void *cb_data)\n-\tconst char *prefix = (const char *)cb_data;\n+\tconst char *prefix = cb_data;\ndiff --git a/thread-utils.c b/thread-utils.c\nindex 6d3de821473..d00d018759b 100644\n--- a/thread-utils.c\n+++ b/thread-utils.c\n@@ -89 +89 @@ int dummy_pthread_create(pthread_t *pthread, const void *attr,\n-\t\t\t void *(*fn)(void *), void *data)\n+\t\t\t void *(*fn), void *data)\ndiff --git a/thread-utils.h b/thread-utils.h\nindex 4961487ed91..e103cf82b0c 100644\n--- a/thread-utils.h\n+++ b/thread-utils.h\n@@ -46 +46 @@ int dummy_pthread_create(pthread_t *pthread, const void *attr,\n-\t\t\t void *(*fn)(void *), void *data);\n+\t\t\t void *(*fn), void *data);\n@@ -49 +49 @@ int dummy_pthread_join(pthread_t pthread, void **retval);\n-int dummy_pthread_init(void *);\n+int dummy_pthread_init;\ndiff --git a/xdiff/xutils.c b/xdiff/xutils.c\nindex 853f2260a1d..a6a6277fb52 100644\n--- a/xdiff/xutils.c\n+++ b/xdiff/xutils.c\n@@ -115 +115 @@ void *xdl_cha_alloc(chastore_t *cha) {\n-\tdata = (char *) ancur + sizeof(chanode_t) + ancur->icurr;\n+\tdata = ancur + sizeof(chanode_t) + ancur->icurr;\n"},{"id":"469222","messageId":"Y53E7PNu05VTdvXG@coredump.intra.peff.net","threadId":"58949","inReplyTo":"xmqqh6xxv946.fsf@gitster.g","subject":"Re: [PATCH] git: remove unneeded casts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-17T13:32:28Z","receivedAt":"2022-12-17T13:32:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 15, 2022 at 08:49:45AM +0900, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I'll stop here for now. It's a fair bit of leg-work digging\n> > these up (though again, I do think there's value in\n> > understanding why the cast was there, even if we know it\n> > isn't _currently_ doing anything).\n> \n> I agree with the value of understanding why each of these casts has\n> become unnecessary, and thanks for demonstrating how a rerolled\n> version should justify its changes with its findings behind each of\n> the unnecessary casts.\n\nI almost didn't write my earlier email, because I was worried that I was\ncreating work where it didn't need to exist (and all of the cases I\nlooked at seemed like good changes). But the cases that Phillip found\nare a good reminder of the value of being careful here.\n\n> What do you recommend the next round should look like?  Multi patch\n> series, each of which removes one cast with its proposed log message\n> explains how it has become unneeded?  A single patch with a gigantic\n> proposed log message that lists the findings for each and all casts\n> that are removed?  Somewhere in between, perhaps split along the\n> file boundary, or grouped by the event that made them unneeded\n> (e.g. \"cmd_main() used to take non-const but when we made it to take\n> const, all of these casts we remove in this patch became unneeded\")?\n\nWhen I've done fixes across the code base like this in the past, I'd\nusually try to group them into patches by root cause. So perhaps:\n\n  - this used to be \"char **argv\" because of main(), but either because\n    of becoming a builtin or cmd_main() it is now const\n\n  - this used to require casting from non-const to const to free()\n\n  - etc, with one-offs for cases that don't fit in any group\n\nThe keeps a single patch's explanation from being too overwhelming, and\navoids repeating yourself when the same logic applies to multiple cases.\n\nThat said, I am happy with any solution that explains why we are sure\neach case is OK to do. :)\n\n-Peff\n"},{"id":"469223","messageId":"Y53KNbX7SO+FOoKf@coredump.intra.peff.net","threadId":"58949","inReplyTo":"221215.86edt12bte.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] git: remove unneeded casts","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-17T13:55:01Z","receivedAt":"2022-12-17T13:55:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 15, 2022 at 11:07:24AM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> Excluding compat/ the diff is the below. Some of it's broken, but some\n> of it suggests things that are worth picking up, e.g. the return value\n> of xmalloc() being cast (a C++-ism), \"(char *)NULL\" (do we ever need to\n> cast NULL?)\n\nYeah, getting rid of explicit casts to/from void (as in xmalloc()) is a\ngood thing, IMHO.\n\nCasting NULL is trickier. In normal assignment, no, it should never be\nneeded. When passed to a variadic function (which covers all the cases\nin your patch below), the compiler needs to know that it's a pointer,\nand not the integer 0. So of the two allowed null pointer constants,\nonly one is guaranteed to work:\n\n  execl(foo, 0); /* bad! there's nothing to say it's a pointer */\n  execl(foo, (char *)0); /* ok */\n\nthough whether it matters in practice depends on your ABI, I think.\n\nBut what about NULL? My copy of C99 (7.17.3) says it \"expands to an\nimplementation-defined null pointer constant\". So it could be either of\nthose.\n\nAnd that is backed up by looking at \"git log -S')NULL'\", which yields\n5d314759d7 (Cast execl*() NULL sentinels to (char *), 2010-07-24).\n\nThat said, I think we have lots of bare NULLs passed to variadic\nfunctions. Any function with LAST_ARG_MUST_BE_NULL will have a NULL in\neach of its callers, and we do not bother casting most of them. So I\nthink this is one of those \"technically could violate the standard, but\nOK in practice\" things.\n\n-Peff\n"},{"id":"469256","messageId":"xmqqo7s0urpe.fsf@gitster.g","threadId":"58949","inReplyTo":"Y53E7PNu05VTdvXG@coredump.intra.peff.net","subject":"Re: [PATCH] git: remove unneeded casts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-19T01:07:09Z","receivedAt":"2022-12-19T01:07:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> ... So perhaps:\n>\n>   - this used to be \"char **argv\" because of main(), but either because\n>     of becoming a builtin or cmd_main() it is now const\n>\n>   - this used to require casting from non-const to const to free()\n>\n>   - etc, with one-offs for cases that don't fit in any group\n>\n> The keeps a single patch's explanation from being too overwhelming, and\n> avoids repeating yourself when the same logic applies to multiple cases.\n\nThanks, that would be the organization I would use, too, if I were\ndoing this change myself.\n\n> That said, I am happy with any solution that explains why we are sure\n> each case is OK to do. :)\n\nYup.\n"}]}