Hi Peff,
On Fri, Oct 19, 2018 at 10:18 AM Jeff King <peff@xxxxxxxx> wrote:
>
> Hi Elijah,
>
> Playing with -Wunused-parameters, I notice there are several functions
> with unused parameters in merge-recursive.c. The patch below drops them
> all. We know they're not being used, so it can't make anything _worse_,
> but this is a good opportunity to confirm that they don't represent some
> latent bug.
>
> In most cases I've been trying to determine the "bug versus cruft" thing
> myself, but I fear that merge-recursive exceeds my abilities here. ;)
These ones all look like cruft to me. I dug through them and tried
looking through history and old submissions for my guesses and how
they ended up here; details below.
> ---
> merge-recursive.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index c0fb83d285..f6d634c3a2 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1369,8 +1369,7 @@ static int merge_mode_and_contents(struct merge_options *o,
>
> static int handle_rename_via_dir(struct merge_options *o,
> struct diff_filepair *pair,
> - const char *rename_branch,
> - const char *other_branch)
> + const char *rename_branch)
Given the similarity in function signature to handle_rename_delete(),
it's possible I copied the function and then started editing. Whether
I was lazily doing that, or if I really added that parameter because I
thought I was going to add an informational message to the user that
used it, or something else, I don't know. But I agree, it's just not
needed and could be added back later if someone did find a use for it.
> @@ -2071,8 +2070,7 @@ static void handle_directory_level_conflicts(struct merge_options *o,
> -static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs,
> - struct tree *tree)
> +static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs)
Yeah, nuke it. I don't remember full details here (it's been over a
year), but my best guess is that I started with (at least part of the)
code for handle_directory_level_conflicts() inside of
get_directory_renames() and then broke it out into a separate function
when it got bigger, but forgot to remove the extra argument.
> {
> struct hashmap *dir_renames;
> struct hashmap_iter iter;
> @@ -2318,8 +2316,7 @@ static void apply_directory_rename_modifications(struct merge_options *o,
> struct tree *o_tree,
> struct tree *a_tree,
> struct tree *b_tree,
> - struct string_list *entries,
> - int *clean)
> + struct string_list *entries)
Yeah, there's several other functions with such a flag; I probably
added it thinking this function would need it too and then just forgot
to remove it when it turned out to not be necessary.
The remaining chunks in the patch you emailed are just modifying
callers to not pass the extra non-unnecessary args, so they're all
good.
