Subject: Re: [RFC 7/7] upload-pack: ack version 2

From: Brandon Williams <[email-masked]>
Date: Fri, 1 Sep 2017 16:20:12 -0700

On 09/01, Bryan Turner wrote:
> On Thu, Aug 24, 2017 at 3:53 PM, Brandon Williams <[email-masked]> wrote:
> > +
> > +       version = getenv("GIT_PROTOCOL");
> > +       if (!strcmp(version, "2"))
> > +               upload_pack_v2();
> > +
> 
> I think the "if" check here needs some type of NULL check for
> "version" before calling "strcmp". Without that, if the "GIT_PROTOCOL"
> environment variable isn't set, git-upload-pack SEGVs.
> 
> This came up when I was testing the "protocol-v2" branch with
> Bitbucket Server. For performance reasons, we sometimes run ref
> advertisements as a separate process, when serving fetches, to allow
> throttling the ref advertisement separately from actually generating
> and sending a packfile. Since CI servers continuously poll for
> changes, but usually don't find any, we want to be able to serve ref
> advertisements, which have minimal overhead, with much higher
> parallelism than serving packs. To do that, we run "git-upload-pack
> --advertize-refs" directly, and that command has never *required*
> "GIT_PROTOCOL" before this change.

Thanks for pointing this out.  Since this was an RFC I wasn't being
careful with doing these sorts of checks :).  I'm currently working on
the non-RFC version of this series and it is getting close to being in a
state where I can send it out for more careful review.

-- 
Brandon Williams