Re: [PATCH 14/15] upload-pack.c: avoid enumerating hidden refs where possible
- From
Taylor Blau <me@ttaylorr.com>
- Date
- May 9, 2023, 21:34 UTC
- Message-ID
- <ZFq8aYppuhjlkChK@nand.local>
- In-Reply-To
- <ZFpjpsmCxDTUG-z3@ncase>
On Tue, May 09, 2023 at 05:15:50PM +0200, Patrick Steinhardt wrote:
Show 20 quoted lines
> On Mon, May 08, 2023 at 06:00:26PM -0400, Taylor Blau wrote:
> > @@ -601,11 +601,32 @@ static int get_common_commits(struct upload_pack_data *data,
> > }
> > }
> >
> > +static int allow_hidden_refs(enum allow_uor allow_uor)
> > +{
> > + return allow_uor & (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1);
> > +}
> > +
> > +static void for_each_namespaced_ref_1(each_ref_fn fn,
> > + struct upload_pack_data *data)
>
> I know it's common practice in the Git project, but personally I tend to
> fight with functions that have a `_1` suffix. You simply cannot tell
> what the difference is to the non-suffixed variant without checking its
> declaration.
>
> `for_each_namespaced_ref_with_optional_hidden_refs()` is definitely a
> mouthful though, and I can't really think of something better either.Yeah, I know. It's not my favorite convention, either, but I also couldn't come up with anything shorter ;-).
Show 8 quoted lines
> > +{
> > + /*
> > + * If `data->allow_uor` allows updating hidden refs, we need to
> > + * mark all references (including hidden ones), to check in
> > + * `is_our_ref()` below.
>
> Doesn't this influence whether somebody can _fetch_ objects pointed to
> by the hidden refs instead of _updating_ them?Oops, yes. Thanks for catching my obvious typo.
Thanks, Taylor