Skip to content

Commit 12a58f9

Browse files
peffgitster
authored andcommitted
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 f1d0190 (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 <[email protected]> Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ee610f0 commit 12a58f9

File tree

3 files changed

+9
-19
lines changed

3 files changed

+9
-19
lines changed

xdiff/xdiffi.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
321321
return -1;
322322

323323
if (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF) {
324-
res = xdl_do_patience_diff(mf1, mf2, xpp, xe);
324+
res = xdl_do_patience_diff(xpp, xe);
325325
goto out;
326326
}
327327

xdiff/xdiffi.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,7 @@ int xdl_build_script(xdfenv_t *xe, xdchange_t **xscr);
5656
void xdl_free_script(xdchange_t *xscr);
5757
int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
5858
xdemitconf_t const *xecfg);
59-
int xdl_do_patience_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
60-
xdfenv_t *env);
59+
int xdl_do_patience_diff(xpparam_t const *xpp, xdfenv_t *env);
6160
int xdl_do_histogram_diff(xpparam_t const *xpp, xdfenv_t *env);
6261

6362
#endif /* #if !defined(XDIFFI_H) */

xdiff/xpatience.c

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ struct hashmap {
6969
} *entries, *first, *last;
7070
/* were common records found? */
7171
unsigned long has_matches;
72-
mmfile_t *file1, *file2;
7372
xdfenv_t *env;
7473
xpparam_t const *xpp;
7574
};
@@ -139,13 +138,10 @@ static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map,
139138
*
140139
* It is assumed that env has been prepared using xdl_prepare().
141140
*/
142-
static int fill_hashmap(mmfile_t *file1, mmfile_t *file2,
143-
xpparam_t const *xpp, xdfenv_t *env,
141+
static int fill_hashmap(xpparam_t const *xpp, xdfenv_t *env,
144142
struct hashmap *result,
145143
int line1, int count1, int line2, int count2)
146144
{
147-
result->file1 = file1;
148-
result->file2 = file2;
149145
result->xpp = xpp;
150146
result->env = env;
151147

@@ -254,8 +250,7 @@ static int match(struct hashmap *map, int line1, int line2)
254250
return record1->ha == record2->ha;
255251
}
256252

257-
static int patience_diff(mmfile_t *file1, mmfile_t *file2,
258-
xpparam_t const *xpp, xdfenv_t *env,
253+
static int patience_diff(xpparam_t const *xpp, xdfenv_t *env,
259254
int line1, int count1, int line2, int count2);
260255

261256
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,
286281

287282
/* Recurse */
288283
if (next1 > line1 || next2 > line2) {
289-
if (patience_diff(map->file1, map->file2,
290-
map->xpp, map->env,
284+
if (patience_diff(map->xpp, map->env,
291285
line1, next1 - line1,
292286
line2, next2 - line2))
293287
return -1;
@@ -326,8 +320,7 @@ static int fall_back_to_classic_diff(struct hashmap *map,
326320
*
327321
* This function assumes that env was prepared with xdl_prepare_env().
328322
*/
329-
static int patience_diff(mmfile_t *file1, mmfile_t *file2,
330-
xpparam_t const *xpp, xdfenv_t *env,
323+
static int patience_diff(xpparam_t const *xpp, xdfenv_t *env,
331324
int line1, int count1, int line2, int count2)
332325
{
333326
struct hashmap map;
@@ -346,7 +339,7 @@ static int patience_diff(mmfile_t *file1, mmfile_t *file2,
346339
}
347340

348341
memset(&map, 0, sizeof(map));
349-
if (fill_hashmap(file1, file2, xpp, env, &map,
342+
if (fill_hashmap(xpp, env, &map,
350343
line1, count1, line2, count2))
351344
return -1;
352345

@@ -374,9 +367,7 @@ static int patience_diff(mmfile_t *file1, mmfile_t *file2,
374367
return result;
375368
}
376369

377-
int xdl_do_patience_diff(mmfile_t *file1, mmfile_t *file2,
378-
xpparam_t const *xpp, xdfenv_t *env)
370+
int xdl_do_patience_diff(xpparam_t const *xpp, xdfenv_t *env)
379371
{
380-
return patience_diff(file1, file2, xpp, env,
381-
1, env->xdf1.nrec, 1, env->xdf2.nrec);
372+
return patience_diff(xpp, env, 1, env->xdf1.nrec, 1, env->xdf2.nrec);
382373
}

0 commit comments

Comments
 (0)