From f1d019071ea68cb2bd2602b925d7a3726ded22e9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Aug 2022 04:49:41 -0400 Subject: xdiff: drop unused mmfile parameters from xdl_do_histogram_diff() These are no longer used since 9df0fc3d57 (xdiff: fix a memory leak, 2022-02-16), as the caller is expected to call xdl_prepare_env() itself. After that change the histogram code only examines the prepared xdfenv_t, not the original buffers. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- xdiff/xdiffi.c | 2 +- xdiff/xdiffi.h | 3 +-- xdiff/xhistogram.c | 3 +-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c index 53e803e6bc..8c64519eac 100644 --- a/xdiff/xdiffi.c +++ b/xdiff/xdiffi.c @@ -326,7 +326,7 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, } if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF) { - res = xdl_do_histogram_diff(mf1, mf2, xpp, xe); + res = xdl_do_histogram_diff(xpp, xe); goto out; } diff --git a/xdiff/xdiffi.h b/xdiff/xdiffi.h index 8f1c7c8b04..9d988e0263 100644 --- a/xdiff/xdiffi.h +++ b/xdiff/xdiffi.h @@ -58,7 +58,6 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, xdemitconf_t const *xecfg); int xdl_do_patience_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdfenv_t *env); -int xdl_do_histogram_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, - xdfenv_t *env); +int xdl_do_histogram_diff(xpparam_t const *xpp, xdfenv_t *env); #endif /* #if !defined(XDIFFI_H) */ diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c index df909004c1..16a8fe2f3f 100644 --- a/xdiff/xhistogram.c +++ b/xdiff/xhistogram.c @@ -362,8 +362,7 @@ out: return result; } -int xdl_do_histogram_diff(mmfile_t *file1, mmfile_t *file2, - xpparam_t const *xpp, xdfenv_t *env) +int xdl_do_histogram_diff(xpparam_t const *xpp, xdfenv_t *env) { return histogram_diff(xpp, env, env->xdf1.dstart + 1, env->xdf1.dend - env->xdf1.dstart + 1, -- cgit v1.2.3 From e2841f706ecf449c67514c253d80445b9db9a08b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Aug 2022 04:50:33 -0400 Subject: log-tree: drop unused commit param in remerge_diff() This function has never used its "commit" parameter since it was added in db757e8b8d (show, log: provide a --remerge-diff capability, 2022-02-02). This makes sense; we already have separate parameters for the parents (which lets us redo the merge) and the oid of the result tree (which we can then diff against the remerge result). Let's drop the unused parameter in the name of clarity. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- log-tree.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/log-tree.c b/log-tree.c index d0ac0a6327..82d9b5f650 100644 --- a/log-tree.c +++ b/log-tree.c @@ -956,8 +956,7 @@ static void cleanup_additional_headers(struct diff_options *o) static int do_remerge_diff(struct rev_info *opt, struct commit_list *parents, - struct object_id *oid, - struct commit *commit) + struct object_id *oid) { struct merge_options o; struct commit_list *bases; @@ -1052,7 +1051,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log "for octopus merges.\n"); return 1; } - return do_remerge_diff(opt, parents, oid, commit); + return do_remerge_diff(opt, parents, oid); } if (opt->combine_merges) return do_diff_combined(opt, commit); -- cgit v1.2.3 From 77651c032c39897ebeba88649d966a9f732facd3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Aug 2022 04:50:54 -0400 Subject: match_pathname(): drop unused "flags" parameter This field has not been used since the function was introduced in b559263216 (exclude: split pathname matching code into a separate function, 2012-10-15), though there was a brief period where it was erroneously used and then reverted in ed4958477b (dir: fix pattern matching on dirs, 2021-09-24) and 5ceb663e92 (dir: fix directory-matching bug, 2021-11-02). It's possible we'd eventually add a flag that makes it useful here, but there are only a handful of callers. It would be easy to add back if necessary, and in the meantime this makes the function interface less misleading. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- attr.c | 2 +- dir.c | 6 ++---- dir.h | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/attr.c b/attr.c index 21e4ad25ad..8a78dde69e 100644 --- a/attr.c +++ b/attr.c @@ -1023,7 +1023,7 @@ static int path_matches(const char *pathname, int pathlen, } return match_pathname(pathname, pathlen - isdir, base, baselen, - pattern, prefix, pat->patternlen, pat->flags); + pattern, prefix, pat->patternlen); } static int macroexpand_one(struct all_attrs_item *all_attrs, int nr, int rem); diff --git a/dir.c b/dir.c index d7cfb08e44..50eeb8b11e 100644 --- a/dir.c +++ b/dir.c @@ -1244,8 +1244,7 @@ int match_basename(const char *basename, int basenamelen, int match_pathname(const char *pathname, int pathlen, const char *base, int baselen, - const char *pattern, int prefix, int patternlen, - unsigned flags) + const char *pattern, int prefix, int patternlen) { const char *name; int namelen; @@ -1347,8 +1346,7 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname if (match_pathname(pathname, pathlen, pattern->base, pattern->baselen ? pattern->baselen - 1 : 0, - exclude, prefix, pattern->patternlen, - pattern->flags)) { + exclude, prefix, pattern->patternlen)) { res = pattern; break; } diff --git a/dir.h b/dir.h index 7bc862030c..674747d93a 100644 --- a/dir.h +++ b/dir.h @@ -414,7 +414,7 @@ int match_basename(const char *, int, const char *, int, int, unsigned); int match_pathname(const char *, int, const char *, int, - const char *, int, int, unsigned); + const char *, int, int); struct path_pattern *last_matching_pattern(struct dir_struct *dir, struct index_state *istate, -- cgit v1.2.3 From 5db8e59cf10bdf5b3a8d4b7cf5c20ee912c2a63a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 20 Aug 2022 05:02:48 -0400 Subject: verify_one_sparse(): drop unused parameters This function has never used its repository or cache_tree parameters since it was introduced in 9ad2d5ea71 (sparse-index: loose integration with cache_tree_verify(), 2021-03-30). As that commit notes, it may eventually be extended further, and that might require looking at more data. But we can easily add them back if necessary (and the repository is even included in the index_state these days already). In the mean time, dropping them makes the code shorter and appeases -Wunused-parameter. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache-tree.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cache-tree.c b/cache-tree.c index 56db0b5026..c97111cccf 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -857,9 +857,7 @@ int cache_tree_matches_traversal(struct cache_tree *root, return 0; } -static void verify_one_sparse(struct repository *r, - struct index_state *istate, - struct cache_tree *it, +static void verify_one_sparse(struct index_state *istate, struct strbuf *path, int pos) { @@ -910,7 +908,7 @@ static int verify_one(struct repository *r, return 1; if (pos >= 0) { - verify_one_sparse(r, istate, it, path, pos); + verify_one_sparse(istate, path, pos); return 0; } -- cgit v1.2.3 From 21a40847ed4a5ec96b0d9cc87b3fe406e7f8f904 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Aug 2022 04:54:56 -0400 Subject: reftable: drop unused parameter from reader_seek_linear() The reader code passes around a "struct reftable_reader" context variable. But the seek function doesn't need it; the table iterator we already get is sufficient. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- reftable/reader.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/reftable/reader.c b/reftable/reader.c index 54b4025105..b4db23ce18 100644 --- a/reftable/reader.c +++ b/reftable/reader.c @@ -443,7 +443,7 @@ static int reader_start(struct reftable_reader *r, struct table_iter *ti, return reader_table_iter_at(r, ti, off, typ); } -static int reader_seek_linear(struct reftable_reader *r, struct table_iter *ti, +static int reader_seek_linear(struct table_iter *ti, struct reftable_record *want) { struct reftable_record rec = @@ -510,7 +510,7 @@ static int reader_seek_indexed(struct reftable_reader *r, if (err < 0) goto done; - err = reader_seek_linear(r, &index_iter, &want_index); + err = reader_seek_linear(&index_iter, &want_index); while (1) { err = table_iter_next(&index_iter, &index_result); table_iter_block_done(&index_iter); @@ -570,7 +570,7 @@ static int reader_seek_internal(struct reftable_reader *r, err = reader_start(r, &ti, reftable_record_type(rec), 0); if (err < 0) return err; - err = reader_seek_linear(r, &ti, rec); + err = reader_seek_linear(&ti, rec); if (err < 0) return err; else { -- cgit v1.2.3 From ee610f00e2834dbfbf4b559e6cce866e8f2aecbd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 Aug 2022 04:55:02 -0400 Subject: reflog: assert PARSE_OPT_NONEG in parse-options callbacks In the spirit of 517fe807d6 (assert NOARG/NONEG behavior of parse-options callbacks, 2018-11-05), this asserts that our callbacks were invoked using the right flags (since otherwise they'd segfault on the NULL arg). Both cases are already correct here, so this is mostly about annotating the functions, and appeasing -Wunused-parameters. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/reflog.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/builtin/reflog.c b/builtin/reflog.c index 4dd297dce8..8123956847 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -193,6 +193,8 @@ static int expire_unreachable_callback(const struct option *opt, { struct cmd_reflog_expire_cb *cmd = opt->value; + BUG_ON_OPT_NEG(unset); + if (parse_expiry_date(arg, &cmd->expire_unreachable)) die(_("invalid timestamp '%s' given to '--%s'"), arg, opt->long_name); @@ -207,6 +209,8 @@ static int expire_total_callback(const struct option *opt, { struct cmd_reflog_expire_cb *cmd = opt->value; + BUG_ON_OPT_NEG(unset); + if (parse_expiry_date(arg, &cmd->expire_total)) die(_("invalid timestamp '%s' given to '--%s'"), arg, opt->long_name); -- cgit v1.2.3 From 12a58f9014e037b4dcee5e427844461b960151d3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 20 Aug 2022 03:36:25 -0400 Subject: xdiff: drop unused mmfile parameters from xdl_do_patience_diff() The entry point to the patience-diff algorithm takes two mmfile_t structs with the original file contents, but it doesn't actually do anything useful with them. This is similar to the case recently cleaned up in the histogram code via f1d019071e (xdiff: drop unused mmfile parameters from xdl_do_histogram_diff(), 2022-08-19), but there's a bit more subtlety going on. We pass them into the recursive patience_diff(), which in turn passes them into fill_hashmap(), which stuffs the pointers into a struct. But the only thing which reads the struct fields is our recursion into patience_diff()! So it's unlikely that something like -Wunused-parameter could find this case: it would have to detect the circular dependency caused by the recursion (not to mention tracing across struct field assignments). But once found, it's easy to have the compiler confirm what's going on: 1. Drop the "file1" and "file2" fields from the hashmap struct definition. Remove the assignments in fill_hashmap(), and temporarily substitute NULL in the recursive call to patience_diff(). Compiling shows that no other code touched those fields. 2. Now fill_hashmap() will trigger -Wunused-parameter. Drop "file1" and "file2" from its definition and callsite. 3. Now patience_diff() will trigger -Wunused-parameter. Drop them there, too. One of the callsites is the recursion with our NULL values, so those temporary values go away. 4. Now xdl_do_patience_diff() will trigger -Wunused-parameter. Drop them there. And we're done. Suggested-by: Phillip Wood Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- xdiff/xdiffi.c | 2 +- xdiff/xdiffi.h | 3 +-- xdiff/xpatience.c | 23 +++++++---------------- 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c index 8c64519eac..32652ded2d 100644 --- a/xdiff/xdiffi.c +++ b/xdiff/xdiffi.c @@ -321,7 +321,7 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, return -1; if (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF) { - res = xdl_do_patience_diff(mf1, mf2, xpp, xe); + res = xdl_do_patience_diff(xpp, xe); goto out; } diff --git a/xdiff/xdiffi.h b/xdiff/xdiffi.h index 9d988e0263..126c9d8ff4 100644 --- a/xdiff/xdiffi.h +++ b/xdiff/xdiffi.h @@ -56,8 +56,7 @@ int xdl_build_script(xdfenv_t *xe, xdchange_t **xscr); void xdl_free_script(xdchange_t *xscr); int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, xdemitconf_t const *xecfg); -int xdl_do_patience_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, - xdfenv_t *env); +int xdl_do_patience_diff(xpparam_t const *xpp, xdfenv_t *env); int xdl_do_histogram_diff(xpparam_t const *xpp, xdfenv_t *env); #endif /* #if !defined(XDIFFI_H) */ diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c index fe39c2978c..a2d8955537 100644 --- a/xdiff/xpatience.c +++ b/xdiff/xpatience.c @@ -69,7 +69,6 @@ struct hashmap { } *entries, *first, *last; /* were common records found? */ unsigned long has_matches; - mmfile_t *file1, *file2; xdfenv_t *env; xpparam_t const *xpp; }; @@ -139,13 +138,10 @@ static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map, * * It is assumed that env has been prepared using xdl_prepare(). */ -static int fill_hashmap(mmfile_t *file1, mmfile_t *file2, - xpparam_t const *xpp, xdfenv_t *env, +static int fill_hashmap(xpparam_t const *xpp, xdfenv_t *env, struct hashmap *result, int line1, int count1, int line2, int count2) { - result->file1 = file1; - result->file2 = file2; result->xpp = xpp; result->env = env; @@ -254,8 +250,7 @@ static int match(struct hashmap *map, int line1, int line2) return record1->ha == record2->ha; } -static int patience_diff(mmfile_t *file1, mmfile_t *file2, - xpparam_t const *xpp, xdfenv_t *env, +static int patience_diff(xpparam_t const *xpp, xdfenv_t *env, int line1, int count1, int line2, int count2); static int walk_common_sequence(struct hashmap *map, struct entry *first, @@ -286,8 +281,7 @@ static int walk_common_sequence(struct hashmap *map, struct entry *first, /* Recurse */ if (next1 > line1 || next2 > line2) { - if (patience_diff(map->file1, map->file2, - map->xpp, map->env, + if (patience_diff(map->xpp, map->env, line1, next1 - line1, line2, next2 - line2)) return -1; @@ -326,8 +320,7 @@ static int fall_back_to_classic_diff(struct hashmap *map, * * This function assumes that env was prepared with xdl_prepare_env(). */ -static int patience_diff(mmfile_t *file1, mmfile_t *file2, - xpparam_t const *xpp, xdfenv_t *env, +static int patience_diff(xpparam_t const *xpp, xdfenv_t *env, int line1, int count1, int line2, int count2) { struct hashmap map; @@ -346,7 +339,7 @@ static int patience_diff(mmfile_t *file1, mmfile_t *file2, } memset(&map, 0, sizeof(map)); - if (fill_hashmap(file1, file2, xpp, env, &map, + if (fill_hashmap(xpp, env, &map, line1, count1, line2, count2)) return -1; @@ -374,9 +367,7 @@ static int patience_diff(mmfile_t *file1, mmfile_t *file2, return result; } -int xdl_do_patience_diff(mmfile_t *file1, mmfile_t *file2, - xpparam_t const *xpp, xdfenv_t *env) +int xdl_do_patience_diff(xpparam_t const *xpp, xdfenv_t *env) { - return patience_diff(file1, file2, xpp, env, - 1, env->xdf1.nrec, 1, env->xdf2.nrec); + return patience_diff(xpp, env, 1, env->xdf1.nrec, 1, env->xdf2.nrec); } -- cgit v1.2.3