All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Elijah Newren <newren@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 3/2] ll_union_merge(): rename path_unused parameter
Date: Thu, 10 Jun 2021 12:14:12 -0400	[thread overview]
Message-ID: <YMI6VADKYmK+aV/C@coredump.intra.peff.net> (raw)
In-Reply-To: <CABPp-BFu0dJa=KxVtzzDG0RrcPo6bGCVx8P8VhB9sV8OjaYQNQ@mail.gmail.com>

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 --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);
 }
-- 
2.32.0.529.g079a794268


  reply	other threads:[~2021-06-10 16:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-10 12:51 [PATCH 0/2] fix union merge with binary files Jeff King
2021-06-10 12:57 ` [PATCH 1/2] ll_binary_merge(): handle XDL_MERGE_FAVOR_UNION Jeff King
2021-06-11  3:36   ` Junio C Hamano
2021-06-10 12:58 ` [PATCH 2/2] ll_union_merge(): pass name labels to ll_xdl_merge() Jeff King
2021-06-10 15:19   ` Elijah Newren
2021-06-10 16:14     ` Jeff King [this message]
2021-06-10 15:19 ` [PATCH 0/2] fix union merge with binary files Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YMI6VADKYmK+aV/C@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --subject='Re: [PATCH 3/2] ll_union_merge(): rename path_unused parameter' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.