development/git
development/git
1
0
Fork 0
mirror of https://github.com/git/git synced 2024-11-04 16:17:49 +00:00
Code Issues Projects Releases Packages Wiki Activity

xdiff: drop unused mmfile parameters from xdl_do_patience_diff()

Browse source 
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 <phillip.wood@dunelm.org.uk>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Jeff King 2022-08-20 03:36:25 -04:00 committed by Junio C Hamano
parent ee610f00e2
commit 12a58f9014
3 changed files with 9 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
xdiff/xdiffi.c
View file

@ -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;
}

3
xdiff/xdiffi.h
View file

@ -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) */

23
xdiff/xpatience.c
View file

@ -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);
}