diff mbox series

[3/2] ll_union_merge(): rename path_unused parameter

Message ID YMI6VADKYmK+aV/C@coredump.intra.peff.net (mailing list archive)
State Accepted
Commit 382b601acde12b298bb84faa11b3f42868716a0d
Headers show
Series fix union merge with binary files | expand

Commit Message

Jeff King June 10, 2021, 4:14 p.m. UTC
On Thu, Jun 10, 2021 at 08:19:02AM -0700, Elijah Newren wrote:

> > Signed-off-by: Jeff King <peff@peff.net>
> > ---
> >  ll-merge.c            | 2 +-
> >  t/t6406-merge-attr.sh | 3 ++-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/ll-merge.c b/ll-merge.c
> > index 145deb12fa..0ee34d8a01 100644
> > --- a/ll-merge.c
> > +++ b/ll-merge.c
> > @@ -151,7 +151,7 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,
> >         o = *opts;
> >         o.variant = XDL_MERGE_FAVOR_UNION;
> >         return ll_xdl_merge(drv_unused, result, path_unused,
> 
> Should we also rename 'path_unused' to 'path', since it is actually used?

That seems reasonable, but I'd prefer to do it as a separate patch. So
here that is.

Ironically, orig_path was unused both before this patch (where we threw
it away and passed NULL instead) and after (where we pass it into the
xdl merge, but the union merge should ignore it completely). But I
prefer not to go too wild with these kind of annotations, as they can
easily become inaccurate or out of date. If we're passing it, then we
shouldn't make too many assumptions about what xdl_merge() is doing
under the hood.

So we could also rename drv_unused mentioned below, but I didn't here.

-- >8 --
Subject: [PATCH] ll_union_merge(): rename path_unused parameter

The "path" parameter to ll_union_merge() is named "path_unused", since
we don't ourselves use it. But we do pass it to ll_xdl_merge(), which
may look at it (it gets passed to ll_binary_merge(), which may pass it
to warning()). Let's rename it to correct this inaccuracy (both of the
other functions correctly do not call this "unused").

Note that we also pass drv_unused, but it truly is unused by the rest of
the stack (it only exists at all to provide a generic interface that
matches what ll_ext_merge() needs).

Reported-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 ll-merge.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/ll-merge.c b/ll-merge.c
index 0ee34d8a01..261657578c 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -138,7 +138,7 @@  static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 
 static int ll_union_merge(const struct ll_merge_driver *drv_unused,
 			  mmbuffer_t *result,
-			  const char *path_unused,
+			  const char *path,
 			  mmfile_t *orig, const char *orig_name,
 			  mmfile_t *src1, const char *name1,
 			  mmfile_t *src2, const char *name2,
@@ -150,7 +150,7 @@  static int ll_union_merge(const struct ll_merge_driver *drv_unused,
 	assert(opts);
 	o = *opts;
 	o.variant = XDL_MERGE_FAVOR_UNION;
-	return ll_xdl_merge(drv_unused, result, path_unused,
+	return ll_xdl_merge(drv_unused, result, path,
 			    orig, orig_name, src1, name1, src2, name2,
 			    &o, marker_size);
 }