{"thread":{"id":"50502","subject":"[PATCH 0/10] dropping more unused function parameters","startedAt":"2019-02-14T05:47:40Z","lastAt":"2019-02-14T05:56:07Z","messageCount":11,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"369280","messageId":"20190214054736.GA20091@sigill.intra.peff.net","threadId":"50502","inReplyTo":null,"subject":"[PATCH 0/10] dropping more unused function parameters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-14T05:47:37Z","receivedAt":"2019-02-14T05:47:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Here are ten more patches from my exploration of -Wunused-parameters.\nMost of these are quite old and not urgent, but things seem relatively\nquiet now, and they all merge cleanly with \"pu\".\n\nIn a sense they should all be trivial to review, as they only remove\nunused parameters. But the thing to consider is whether that parameter's\nlack of use is actually a bug, or if it's simply cruft (and I argue in\neach case in favor of cruft).\n\n  [01/10]: diff: drop options parameter from diffcore_fix_diff_index()\n  [02/10]: diff: drop unused color reset parameters\n  [03/10]: diff: drop unused emit data parameter from sane_truncate_line()\n  [04/10]: diff: drop complete_rewrite parameter from run_external_diff()\n  [05/10]: merge-recursive: drop several unused parameters\n  [06/10]: pack-objects: drop unused parameter from oe_map_new_pack()\n  [07/10]: files-backend: drop refs parameter from split_symref_update()\n  [08/10]: ref-filter: drop unused buf/sz pairs\n  [09/10]: ref-filter: drop unused \"obj\" parameters\n  [10/10]: ref-filter: drop unused \"sz\" parameters\n\n diff-lib.c           |  2 +-\n diff.c               | 31 ++++++++++++-------------------\n diff.h               |  2 +-\n merge-recursive.c    | 19 +++++++------------\n pack-objects.c       |  3 +--\n pack-objects.h       |  6 +++---\n ref-filter.c         | 36 ++++++++++++++++++------------------\n refs/files-backend.c |  5 ++---\n 8 files changed, 45 insertions(+), 59 deletions(-)\n\n-Peff\n"},{"id":"369281","messageId":"20190214054803.GA20578@sigill.intra.peff.net","threadId":"50502","inReplyTo":"20190214054736.GA20091@sigill.intra.peff.net","subject":"[PATCH 01/10] diff: drop options parameter from diffcore_fix_diff_index()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-14T05:48:03Z","receivedAt":"2019-02-14T05:48:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The sole purpose of this function is to fix the sorting order of the\nqueued diff entries. It doesn't need to know about any diff options, so\nwe can drop the unused parameter.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n diff-lib.c | 2 +-\n diff.c     | 2 +-\n diff.h     | 2 +-\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 23c8d351b3..a838c219ec 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -531,7 +531,7 @@ int run_diff_index(struct rev_info *revs, int cached)\n \t\texit(128);\n \n \tdiff_set_mnemonic_prefix(&revs->diffopt, \"c/\", cached ? \"i/\" : \"w/\");\n-\tdiffcore_fix_diff_index(&revs->diffopt);\n+\tdiffcore_fix_diff_index();\n \tdiffcore_std(&revs->diffopt);\n \tdiff_flush(&revs->diffopt);\n \ttrace_performance_leave(\"diff-index\");\ndiff --git a/diff.c b/diff.c\nindex 5306c48652..3e7d11fb70 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -6224,7 +6224,7 @@ static int diffnamecmp(const void *a_, const void *b_)\n \treturn strcmp(name_a, name_b);\n }\n \n-void diffcore_fix_diff_index(struct diff_options *options)\n+void diffcore_fix_diff_index(void)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tQSORT(q->queue, q->nr, diffnamecmp);\ndiff --git a/diff.h b/diff.h\nindex b512d0477a..8f55aad5ba 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -367,7 +367,7 @@ int git_config_rename(const char *var, const char *value);\n #define DIFF_PICKAXE_IGNORE_CASE\t32\n \n void diffcore_std(struct diff_options *);\n-void diffcore_fix_diff_index(struct diff_options *);\n+void diffcore_fix_diff_index(void);\n \n #define COMMON_DIFF_OPTIONS_HELP \\\n \"\\ncommon diff options:\\n\" \\\n-- \n2.21.0.rc1.567.g648f076c3f\n\n"},{"id":"369282","messageId":"20190214054813.GB20578@sigill.intra.peff.net","threadId":"50502","inReplyTo":"20190214054736.GA20091@sigill.intra.peff.net","subject":"[PATCH 02/10] diff: drop unused color reset parameters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-14T05:48:13Z","receivedAt":"2019-02-14T05:48:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Several of the emit_* functions take a \"reset\" color parameter, but\nnever actually look at it (instead, they call into emit_diff_symbol,\nwhich handles the colors itself). Let's drop these unused parameters.\n\nNote that emit_line() does still take a color/reset pair, and actually\nuses it. It cannot be refactored to match these other functions because\nit's the thing that emit_diff_symbol eventually calls into (i.e., it\ndoes not by itself know which colors to use, and must be told by the\ncaller).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n diff.c | 21 ++++++++-------------\n 1 file changed, 8 insertions(+), 13 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 3e7d11fb70..de45bbd76e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1613,8 +1613,7 @@ static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line\n \treturn ws_blank_line(line, len, ecbdata->ws_rule);\n }\n \n-static void emit_add_line(const char *reset,\n-\t\t\t  struct emit_callback *ecbdata,\n+static void emit_add_line(struct emit_callback *ecbdata,\n \t\t\t  const char *line, int len)\n {\n \tunsigned flags = WSEH_NEW | ecbdata->ws_rule;\n@@ -1624,16 +1623,14 @@ static void emit_add_line(const char *reset,\n \temit_diff_symbol(ecbdata->opt, DIFF_SYMBOL_PLUS, line, len, flags);\n }\n \n-static void emit_del_line(const char *reset,\n-\t\t\t  struct emit_callback *ecbdata,\n+static void emit_del_line(struct emit_callback *ecbdata,\n \t\t\t  const char *line, int len)\n {\n \tunsigned flags = WSEH_OLD | ecbdata->ws_rule;\n \temit_diff_symbol(ecbdata->opt, DIFF_SYMBOL_MINUS, line, len, flags);\n }\n \n-static void emit_context_line(const char *reset,\n-\t\t\t      struct emit_callback *ecbdata,\n+static void emit_context_line(struct emit_callback *ecbdata,\n \t\t\t      const char *line, int len)\n {\n \tunsigned flags = WSEH_CONTEXT | ecbdata->ws_rule;\n@@ -1742,7 +1739,6 @@ static void emit_rewrite_lines(struct emit_callback *ecb,\n \t\t\t       int prefix, const char *data, int size)\n {\n \tconst char *endp = NULL;\n-\tconst char *reset = diff_get_color(ecb->color_diff, DIFF_RESET);\n \n \twhile (0 < size) {\n \t\tint len;\n@@ -1751,10 +1747,10 @@ static void emit_rewrite_lines(struct emit_callback *ecb,\n \t\tlen = endp ? (endp - data + 1) : size;\n \t\tif (prefix != '+') {\n \t\t\tecb->lno_in_preimage++;\n-\t\t\temit_del_line(reset, ecb, data, len);\n+\t\t\temit_del_line(ecb, data, len);\n \t\t} else {\n \t\t\tecb->lno_in_postimage++;\n-\t\t\temit_add_line(reset, ecb, data, len);\n+\t\t\temit_add_line(ecb, data, len);\n \t\t}\n \t\tsize -= len;\n \t\tdata += len;\n@@ -2325,7 +2321,6 @@ static void find_lno(const char *line, struct emit_callback *ecbdata)\n static void fn_out_consume(void *priv, char *line, unsigned long len)\n {\n \tstruct emit_callback *ecbdata = priv;\n-\tconst char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n \tstruct diff_options *o = ecbdata->opt;\n \n \to->found_changes = 1;\n@@ -2392,16 +2387,16 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \tswitch (line[0]) {\n \tcase '+':\n \t\tecbdata->lno_in_postimage++;\n-\t\temit_add_line(reset, ecbdata, line + 1, len - 1);\n+\t\temit_add_line(ecbdata, line + 1, len - 1);\n \t\tbreak;\n \tcase '-':\n \t\tecbdata->lno_in_preimage++;\n-\t\temit_del_line(reset, ecbdata, line + 1, len - 1);\n+\t\temit_del_line(ecbdata, line + 1, len - 1);\n \t\tbreak;\n \tcase ' ':\n \t\tecbdata->lno_in_postimage++;\n \t\tecbdata->lno_in_preimage++;\n-\t\temit_context_line(reset, ecbdata, line + 1, len - 1);\n+\t\temit_context_line(ecbdata, line + 1, len - 1);\n \t\tbreak;\n \tdefault:\n \t\t/* incomplete line at the end */\n-- \n2.21.0.rc1.567.g648f076c3f\n\n"},{"id":"369283","messageId":"20190214054827.GC20578@sigill.intra.peff.net","threadId":"50502","inReplyTo":"20190214054736.GA20091@sigill.intra.peff.net","subject":"[PATCH 03/10] diff: drop unused emit data parameter from sane_truncate_line()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-14T05:48:27Z","receivedAt":"2019-02-14T05:48:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We pass the \"struct emit_callback\" (which contains all of the context\nfor our diff) into sane_truncate_line(), but that function doesn't\nactually use it. In theory we might eventually develop a diff option\nthat impacts this, but in the meantime let's not mislead anybody reading\nthe code. Since the function is static, it would be easy to pass it\nagain if it should ever become useful.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n diff.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex de45bbd76e..d24843b531 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2287,7 +2287,7 @@ const char *diff_line_prefix(struct diff_options *opt)\n \treturn msgbuf->buf;\n }\n \n-static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, unsigned long len)\n+static unsigned long sane_truncate_line(char *line, unsigned long len)\n {\n \tconst char *cp;\n \tunsigned long allot;\n@@ -2351,7 +2351,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \tif (line[0] == '@') {\n \t\tif (ecbdata->diff_words)\n \t\t\tdiff_words_flush(ecbdata);\n-\t\tlen = sane_truncate_line(ecbdata, line, len);\n+\t\tlen = sane_truncate_line(line, len);\n \t\tfind_lno(line, ecbdata);\n \t\temit_hunk_header(ecbdata, line, len);\n \t\treturn;\n-- \n2.21.0.rc1.567.g648f076c3f\n\n"},{"id":"369285","messageId":"20190214054941.GD20578@sigill.intra.peff.net","threadId":"50502","inReplyTo":"20190214054736.GA20091@sigill.intra.peff.net","subject":"[PATCH 04/10] diff: drop complete_rewrite parameter from run_external_diff()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-14T05:49:42Z","receivedAt":"2019-02-14T05:49:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Our builtin_diff() wants to know whether break-detection found a\ncomplete rewrite, because it changes how the diff is shown. However,\nwhen calling out to an external diff, we don't pass this information\nalong (and doing so would require designing a new interface to the\nuser-provided program).\n\nLet's drop the unused parameter to make this fact more clear.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nAn alternative is to pass $COMPLETE_REWRITE in the environment. That\nwould avoid disrupting existing external diff callers, and they could\npeek at the information if they really cared. But given that nobody has\nactually asked for this, I'm inclined not to add a new interface element\nthat we'd then have to support forever.\n\n diff.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d24843b531..e3f263769b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4178,7 +4178,6 @@ static void run_external_diff(const char *pgm,\n \t\t\t      struct diff_filespec *one,\n \t\t\t      struct diff_filespec *two,\n \t\t\t      const char *xfrm_msg,\n-\t\t\t      int complete_rewrite,\n \t\t\t      struct diff_options *o)\n {\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n@@ -4336,8 +4335,7 @@ static void run_diff_cmd(const char *pgm,\n \t}\n \n \tif (pgm) {\n-\t\trun_external_diff(pgm, name, other, one, two, xfrm_msg,\n-\t\t\t\t  complete_rewrite, o);\n+\t\trun_external_diff(pgm, name, other, one, two, xfrm_msg, o);\n \t\treturn;\n \t}\n \tif (one && two)\n-- \n2.21.0.rc1.567.g648f076c3f\n\n"},{"id":"369286","messageId":"20190214055002.GE20578@sigill.intra.peff.net","threadId":"50502","inReplyTo":"20190214054736.GA20091@sigill.intra.peff.net","subject":"[PATCH 05/10] merge-recursive: drop several unused parameters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-14T05:50:02Z","receivedAt":"2019-02-14T05:50:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are a few functions related to directory renames that have unused\nparameters. After consulting with the author in [1], these seem to be\nleftover cruft from the development process, and not signs of any bug.\nLet's drop them.\n\n[1] https://public-inbox.org/git/CABPp-BHobf8wbBsXF97scNQCzkxQukziODRXq6JOOWq61cAd9g@mail.gmail.com/\n\nHelped-by: Elijah Newren <newren@gmail.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n merge-recursive.c | 19 +++++++------------\n 1 file changed, 7 insertions(+), 12 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 4851825aeb..f270fa66f3 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1402,8 +1402,7 @@ static int merge_mode_and_contents(struct merge_options *o,\n \n static int handle_rename_via_dir(struct merge_options *o,\n \t\t\t\t struct diff_filepair *pair,\n-\t\t\t\t const char *rename_branch,\n-\t\t\t\t const char *other_branch)\n+\t\t\t\t const char *rename_branch)\n {\n \t/*\n \t * Handle file adds that need to be renamed due to directory rename\n@@ -2213,8 +2212,7 @@ static void handle_directory_level_conflicts(struct merge_options *o,\n \tremove_hashmap_entries(dir_re_merge, &remove_from_merge);\n }\n \n-static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs,\n-\t\t\t\t\t     struct tree *tree)\n+static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs)\n {\n \tstruct hashmap *dir_renames;\n \tstruct hashmap_iter iter;\n@@ -2460,8 +2458,7 @@ static void apply_directory_rename_modifications(struct merge_options *o,\n \t\t\t\t\t\t struct tree *o_tree,\n \t\t\t\t\t\t struct tree *a_tree,\n \t\t\t\t\t\t struct tree *b_tree,\n-\t\t\t\t\t\t struct string_list *entries,\n-\t\t\t\t\t\t int *clean)\n+\t\t\t\t\t\t struct string_list *entries)\n {\n \tstruct string_list_item *item;\n \tint stage = (tree == a_tree ? 2 : 3);\n@@ -2632,8 +2629,7 @@ static struct string_list *get_renames(struct merge_options *o,\n \t\t\tapply_directory_rename_modifications(o, pair, new_path,\n \t\t\t\t\t\t\t     re, tree, o_tree,\n \t\t\t\t\t\t\t     a_tree, b_tree,\n-\t\t\t\t\t\t\t     entries,\n-\t\t\t\t\t\t\t     clean_merge);\n+\t\t\t\t\t\t\t     entries);\n \t}\n \n \thashmap_iter_init(&collisions, &iter);\n@@ -2944,8 +2940,8 @@ static int detect_and_process_renames(struct merge_options *o,\n \tmerge_pairs = get_diffpairs(o, common, merge);\n \n \tif (o->detect_directory_renames) {\n-\t\tdir_re_head = get_directory_renames(head_pairs, head);\n-\t\tdir_re_merge = get_directory_renames(merge_pairs, merge);\n+\t\tdir_re_head = get_directory_renames(head_pairs);\n+\t\tdir_re_merge = get_directory_renames(merge_pairs);\n \n \t\thandle_directory_level_conflicts(o,\n \t\t\t\t\t\t dir_re_head, head,\n@@ -3268,8 +3264,7 @@ static int process_entry(struct merge_options *o,\n \t\t\tclean_merge = 1;\n \t\t\tif (handle_rename_via_dir(o,\n \t\t\t\t\t\t  conflict_info->pair1,\n-\t\t\t\t\t\t  conflict_info->branch1,\n-\t\t\t\t\t\t  conflict_info->branch2))\n+\t\t\t\t\t\t  conflict_info->branch1))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_ADD:\n-- \n2.21.0.rc1.567.g648f076c3f\n\n"},{"id":"369287","messageId":"20190214055031.GF20578@sigill.intra.peff.net","threadId":"50502","inReplyTo":"20190214054736.GA20091@sigill.intra.peff.net","subject":"[PATCH 06/10] pack-objects: drop unused parameter from oe_map_new_pack()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-14T05:50:32Z","receivedAt":"2019-02-14T05:50:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since 43fa44fa3b (pack-objects: move in_pack out of struct object_entry,\n2018-04-14), we store the source pack for each object as a small index\nrather than as a pointer. When we see a new pack that has no allocated\nindex, we fall back to generating an array of pointers by calling\noe_map_new_pack().\n\nPerhaps counter-intuitively, that function does not need to actually see\nour new index-less pack. It only allocates and populates the array with\nthe existing packs, after which oe_set_in_pack() actually adds the new\npack to the array.\n\nLet's drop the unused \"struct packed_git\" argument to oe_map_new_pack()\nto avoid confusion.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n pack-objects.c | 3 +--\n pack-objects.h | 6 +++---\n 2 files changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/pack-objects.c b/pack-objects.c\nindex e7cd337bee..ce33b8906e 100644\n--- a/pack-objects.c\n+++ b/pack-objects.c\n@@ -119,8 +119,7 @@ static void prepare_in_pack_by_idx(struct packing_data *pdata)\n  * this fall back code, just stay simple and fall back to using\n  * in_pack[] array.\n  */\n-void oe_map_new_pack(struct packing_data *pack,\n-\t\t     struct packed_git *p)\n+void oe_map_new_pack(struct packing_data *pack)\n {\n \tuint32_t i;\n \ndiff --git a/pack-objects.h b/pack-objects.h\nindex 6bfacc7d2c..6fde7ce27c 100644\n--- a/pack-objects.h\n+++ b/pack-objects.h\n@@ -247,14 +247,14 @@ static inline struct packed_git *oe_in_pack(const struct packing_data *pack,\n \t\treturn pack->in_pack[e - pack->objects];\n }\n \n-void oe_map_new_pack(struct packing_data *pack,\n-\t\t     struct packed_git *p);\n+void oe_map_new_pack(struct packing_data *pack);\n+\n static inline void oe_set_in_pack(struct packing_data *pack,\n \t\t\t\t  struct object_entry *e,\n \t\t\t\t  struct packed_git *p)\n {\n \tif (!p->index)\n-\t\toe_map_new_pack(pack, p);\n+\t\toe_map_new_pack(pack);\n \tif (pack->in_pack_by_idx)\n \t\te->in_pack_idx = p->index;\n \telse\n-- \n2.21.0.rc1.567.g648f076c3f\n\n"},{"id":"369288","messageId":"20190214055045.GG20578@sigill.intra.peff.net","threadId":"50502","inReplyTo":"20190214054736.GA20091@sigill.intra.peff.net","subject":"[PATCH 07/10] files-backend: drop refs parameter from split_symref_update()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-14T05:50:46Z","receivedAt":"2019-02-14T05:50:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This parameter was added in fcc42ea0c9 (split_symref_update(): add a\nfiles_ref_store argument, 2016-09-04) without comment, but never used.\nThe splitting is purely mechanical, and doesn't depend on the particular\nref-store. Let's drop this parameter in the name of simplicity.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n refs/files-backend.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex dd8abe9185..ef053f716c 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2254,8 +2254,7 @@ static int split_head_update(struct ref_update *update,\n  * Note that the new update will itself be subject to splitting when\n  * the iteration gets to it.\n  */\n-static int split_symref_update(struct files_ref_store *refs,\n-\t\t\t       struct ref_update *update,\n+static int split_symref_update(struct ref_update *update,\n \t\t\t       const char *referent,\n \t\t\t       struct ref_transaction *transaction,\n \t\t\t       struct string_list *affected_refnames,\n@@ -2449,7 +2448,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,\n \t\t\t * of processing the split-off update, so we\n \t\t\t * don't have to do it here.\n \t\t\t */\n-\t\t\tret = split_symref_update(refs, update,\n+\t\t\tret = split_symref_update(update,\n \t\t\t\t\t\t  referent.buf, transaction,\n \t\t\t\t\t\t  affected_refnames, err);\n \t\t\tif (ret)\n-- \n2.21.0.rc1.567.g648f076c3f\n\n"},{"id":"369289","messageId":"20190214055054.GH20578@sigill.intra.peff.net","threadId":"50502","inReplyTo":"20190214054736.GA20091@sigill.intra.peff.net","subject":"[PATCH 08/10] ref-filter: drop unused buf/sz pairs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-14T05:50:54Z","receivedAt":"2019-02-14T05:53:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The grab_tag_values() and grab_commit_values() functions take both the\n\"struct object\" as well as the buf/sz pair for the actual object bytes.\nHowever, neither function uses the latter, as they pull the data\ndirectly from the parsed object struct.\n\nLet's get rid of these misleading parameters.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n ref-filter.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 422a9c9ae3..c9333b3181 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -913,7 +913,7 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n }\n \n /* See grab_values */\n-static void grab_tag_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+static void grab_tag_values(struct atom_value *val, int deref, struct object *obj)\n {\n \tint i;\n \tstruct tag *tag = (struct tag *) obj;\n@@ -935,7 +935,7 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob\n }\n \n /* See grab_values */\n-static void grab_commit_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+static void grab_commit_values(struct atom_value *val, int deref, struct object *obj)\n {\n \tint i;\n \tstruct commit *commit = (struct commit *) obj;\n@@ -1269,12 +1269,12 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v\n {\n \tswitch (obj->type) {\n \tcase OBJ_TAG:\n-\t\tgrab_tag_values(val, deref, obj, buf, sz);\n+\t\tgrab_tag_values(val, deref, obj);\n \t\tgrab_sub_body_contents(val, deref, obj, buf, sz);\n \t\tgrab_person(\"tagger\", val, deref, obj, buf, sz);\n \t\tbreak;\n \tcase OBJ_COMMIT:\n-\t\tgrab_commit_values(val, deref, obj, buf, sz);\n+\t\tgrab_commit_values(val, deref, obj);\n \t\tgrab_sub_body_contents(val, deref, obj, buf, sz);\n \t\tgrab_person(\"author\", val, deref, obj, buf, sz);\n \t\tgrab_person(\"committer\", val, deref, obj, buf, sz);\n-- \n2.21.0.rc1.567.g648f076c3f\n\n"},{"id":"369291","messageId":"20190214055058.GI20578@sigill.intra.peff.net","threadId":"50502","inReplyTo":"20190214054736.GA20091@sigill.intra.peff.net","subject":"[PATCH 09/10] ref-filter: drop unused \"obj\" parameters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-14T05:50:58Z","receivedAt":"2019-02-14T05:56:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The grab_person() and grab_sub_body_contents() functions take both an\nobject struct and a buf/sz pair of the object bytes. However, they use\nonly the latter, since \"struct object\" does not contain the parsed ident\n(nor the whole commit message, of course).\n\nLet's get rid of these misleading \"struct object\" parameters. It's\npossible we may want them in the future (e.g., to generate error\nmessages that mention the object id), but since these are static\nfunctions, we can easily add them back in later (and if we do want that\ninformation, it's likely we'd pass it through a more generalized\n\"parsing context\" struct anyway).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n ref-filter.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex c9333b3181..9ba18b220c 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1064,7 +1064,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam\n }\n \n /* See grab_values */\n-static void grab_person(const char *who, struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+static void grab_person(const char *who, struct atom_value *val, int deref, void *buf, unsigned long sz)\n {\n \tint i;\n \tint wholen = strlen(who);\n@@ -1192,7 +1192,7 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size\n }\n \n /* See grab_values */\n-static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf, unsigned long sz)\n {\n \tint i;\n \tconst char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;\n@@ -1270,14 +1270,14 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v\n \tswitch (obj->type) {\n \tcase OBJ_TAG:\n \t\tgrab_tag_values(val, deref, obj);\n-\t\tgrab_sub_body_contents(val, deref, obj, buf, sz);\n-\t\tgrab_person(\"tagger\", val, deref, obj, buf, sz);\n+\t\tgrab_sub_body_contents(val, deref, buf, sz);\n+\t\tgrab_person(\"tagger\", val, deref, buf, sz);\n \t\tbreak;\n \tcase OBJ_COMMIT:\n \t\tgrab_commit_values(val, deref, obj);\n-\t\tgrab_sub_body_contents(val, deref, obj, buf, sz);\n-\t\tgrab_person(\"author\", val, deref, obj, buf, sz);\n-\t\tgrab_person(\"committer\", val, deref, obj, buf, sz);\n+\t\tgrab_sub_body_contents(val, deref, buf, sz);\n+\t\tgrab_person(\"author\", val, deref, buf, sz);\n+\t\tgrab_person(\"committer\", val, deref, buf, sz);\n \t\tbreak;\n \tcase OBJ_TREE:\n \t\t/* grab_tree_values(val, deref, obj, buf, sz); */\n-- \n2.21.0.rc1.567.g648f076c3f\n\n"},{"id":"369292","messageId":"20190214055103.GJ20578@sigill.intra.peff.net","threadId":"50502","inReplyTo":"20190214054736.GA20091@sigill.intra.peff.net","subject":"[PATCH 10/10] ref-filter: drop unused \"sz\" parameters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-14T05:51:03Z","receivedAt":"2019-02-14T05:56:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Many of our grab_* functions, which parse the object content, take a\nbuf/sz pair of the object bytes. However, the functions which actually\nparse the buffers (like find_wholine() and find_subpos()) never look at\n\"sz\", and instead use functions like strchr() and strchrnul() that\nassume the result is NUL-terminated.\n\nThis is OK in practice (and common for Git's parsing code), since we\nalways allocate an extra NUL when loading an object into memory (and\nlikewise, we are OK with stopping parsing if a commit or tag contains an\nembedded NUL).\n\nLet's drop these extra \"sz\" parameters, as they are misleading about how\nthe functions intend to access the buffer. We can drop from both the\nfunctions mentioned above, which in turn lets us drop from their\ncallers, cascading all the way up to the top-level grab_values().\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n ref-filter.c | 28 ++++++++++++++--------------\n 1 file changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 9ba18b220c..09b5fb439f 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -968,7 +968,7 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object\n \t}\n }\n \n-static const char *find_wholine(const char *who, int wholen, const char *buf, unsigned long sz)\n+static const char *find_wholine(const char *who, int wholen, const char *buf)\n {\n \tconst char *eol;\n \twhile (*buf) {\n@@ -1064,7 +1064,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam\n }\n \n /* See grab_values */\n-static void grab_person(const char *who, struct atom_value *val, int deref, void *buf, unsigned long sz)\n+static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)\n {\n \tint i;\n \tint wholen = strlen(who);\n@@ -1085,7 +1085,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t\t    !starts_with(name + wholen, \"date\"))\n \t\t\tcontinue;\n \t\tif (!wholine)\n-\t\t\twholine = find_wholine(who, wholen, buf, sz);\n+\t\t\twholine = find_wholine(who, wholen, buf);\n \t\tif (!wholine)\n \t\t\treturn; /* no point looking for it */\n \t\tif (name[wholen] == 0)\n@@ -1105,7 +1105,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \tif (strcmp(who, \"tagger\") && strcmp(who, \"committer\"))\n \t\treturn; /* \"author\" for commit object is not wanted */\n \tif (!wholine)\n-\t\twholine = find_wholine(who, wholen, buf, sz);\n+\t\twholine = find_wholine(who, wholen, buf);\n \tif (!wholine)\n \t\treturn;\n \tfor (i = 0; i < used_atom_cnt; i++) {\n@@ -1123,7 +1123,7 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void\n \t}\n }\n \n-static void find_subpos(const char *buf, unsigned long sz,\n+static void find_subpos(const char *buf,\n \t\t\tconst char **sub, unsigned long *sublen,\n \t\t\tconst char **body, unsigned long *bodylen,\n \t\t\tunsigned long *nonsiglen,\n@@ -1192,7 +1192,7 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size\n }\n \n /* See grab_values */\n-static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf, unsigned long sz)\n+static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n {\n \tint i;\n \tconst char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;\n@@ -1212,7 +1212,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf,\n \t\t    !starts_with(name, \"contents\"))\n \t\t\tcontinue;\n \t\tif (!subpos)\n-\t\t\tfind_subpos(buf, sz,\n+\t\t\tfind_subpos(buf,\n \t\t\t\t    &subpos, &sublen,\n \t\t\t\t    &bodypos, &bodylen, &nonsiglen,\n \t\t\t\t    &sigpos, &siglen);\n@@ -1265,19 +1265,19 @@ static void fill_missing_values(struct atom_value *val)\n  * pointed at by the ref itself; otherwise it is the object the\n  * ref (which is a tag) refers to.\n  */\n-static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)\n+static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf)\n {\n \tswitch (obj->type) {\n \tcase OBJ_TAG:\n \t\tgrab_tag_values(val, deref, obj);\n-\t\tgrab_sub_body_contents(val, deref, buf, sz);\n-\t\tgrab_person(\"tagger\", val, deref, buf, sz);\n+\t\tgrab_sub_body_contents(val, deref, buf);\n+\t\tgrab_person(\"tagger\", val, deref, buf);\n \t\tbreak;\n \tcase OBJ_COMMIT:\n \t\tgrab_commit_values(val, deref, obj);\n-\t\tgrab_sub_body_contents(val, deref, buf, sz);\n-\t\tgrab_person(\"author\", val, deref, buf, sz);\n-\t\tgrab_person(\"committer\", val, deref, buf, sz);\n+\t\tgrab_sub_body_contents(val, deref, buf);\n+\t\tgrab_person(\"author\", val, deref, buf);\n+\t\tgrab_person(\"committer\", val, deref, buf);\n \t\tbreak;\n \tcase OBJ_TREE:\n \t\t/* grab_tree_values(val, deref, obj, buf, sz); */\n@@ -1516,7 +1516,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj\n \t\t\treturn strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n \t\t\t\t\t       oid_to_hex(&oi->oid), ref->refname);\n \t\t}\n-\t\tgrab_values(ref->value, deref, *obj, oi->content, oi->size);\n+\t\tgrab_values(ref->value, deref, *obj, oi->content);\n \t}\n \n \tgrab_common_values(ref->value, deref, oi);\n-- \n2.21.0.rc1.567.g648f076c3f\n"}]}