From: Jeff King <peff@peff.net>
Date: Thu, 31 Aug 2023 20:48:29 GMT
Subject: Re: [PATCH 4/8] parse-options: mark unused "opt" parameter in callbacks
Message-ID: <20230831204829.GA3233491@coredump.intra.peff.net>
In-Reply-To: <20230831175018.GB3208283@coredump.intra.peff.net>

On Thu, Aug 31, 2023 at 01:50:18PM -0400, Jeff King wrote:

> > > -static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)
> > > +static int parse_refmap_arg(const struct option *opt UNUSED,
> > > +			    const char *arg, int unset)
> > >  {
> > >  	BUG_ON_OPT_NEG(unset);
> > 
> > Can't this just point opt->value at the global &refmap?  Obviously
> > not a huge deal, as we could have taken the "annotate as UNUSED"
> > approach for all the functions in [3/8].
> 
> Hmm, yeah. I think I looked at the abstract refspec_append() here and
> assumed that it might be touching other variables. But it's not. It's
> operating purely on the &refspec we pass it (and even though it uses
> ALLOC_GROW, the "nr" and "alloc" are both contained in the struct). So
> yeah, it really should have been converted in patch 3.

Oh, btw, there are two other cases in this patch that _could_ be
converted but I left alone. In log.c, we have a to_callback() and a
cc_callback(), both of which take a single string list. But there is
also header_callback(), which resets both of those when it sees
--no-add-header. So I left them alone as a set.

However, it occurs to me that to_callback() and cc_callback() can just
be OPT_STRING_LIST these days. And that is probably a worthwhile
cleanup to do.

-Peff