Re: [RFC 7/7] upload-pack: ack version 2
- From
- Brandon Williams <bmwill@google.com>
- Date
- Sep 1, 2017, 23:20 UTC
- Message-ID
- <20170901232012.GA16739@google.com>
- In-Reply-To
- <CAGyf7-GKAW3Dn_rPKxtidKoXh7a6NmDFYzGrb9tGOszhsMignA@mail.gmail.com>
On 09/01, Bryan Turner wrote:
Show 21 quoted lines
> On Thu, Aug 24, 2017 at 3:53 PM, Brandon Williams <bmwill@google.com> 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