Re: [PATCH v10 7/8] transport: add client support for object-info

From: Peijian Ju <eric.peijian@gmail.com>
Date: Feb 20, 2025, 22:52 UTC
Message-ID: <CAN2LT1Dxd6e1q_iE7_M_yf3aAyTwasX4ehRwEK_86YPJa8bkqw@mail.gmail.com>
In-Reply-To: <20250201020806.GA4081995@coredump.intra.peff.net>

On Fri, Jan 31, 2025 at 9:08 PM Jeff King <peff@peff.net> wrote:

Show 21 quoted lines

>
> On Mon, Jan 13, 2025 at 09:14:59PM -0500, Eric Ju wrote:
>
> > @@ -464,8 +465,33 @@ static int fetch_refs_via_pack(struct transport *transport,
> >       args.server_options = transport->server_options;
> >       args.negotiation_tips = data->options.negotiation_tips;
> >       args.reject_shallow_remote = transport->smart_options->reject_shallow;
> > +     args.object_info = transport->smart_options->object_info;
> > +
> > +     if (transport->smart_options && transport->smart_options->object_info
>
> Coverity complains about the check for a NULL transport->smart_options
> here. If it's NULL we'd already have segfaulted a few lines above when
> we look at the reject_shallow flag.
>
> Not sure if that's an existing bug in the earlier code or not. ;) Your
> extra check can't hurt anything, in the sense that it's just being
> overly defensive, but it does make puzzling out the expected value of
> smart_options harder.
>
> -Peff

Thank you Jeff. Sorry for the late response.

I will remove the extra check. transport->smart_options will not be NULL when it reaches `args.reject_shallow_remote = transport->smart_options->reject_shallow;`

The call sequence is like this

get_remote_info() in cat-file.c ==> transport_fetch_refs() ==> transport->vtable->fetch_refs ==> fetch_refs_via_pack()

in get_remote_info(), we already have a check for NULL:

if (gtransport->smart_options) {
  ...
}  else {
        retval = -1;
}