Re: [PATCH 11/11] bug_fl(): add missing `va_end()` call

From: Jeff King <peff@peff.net>
Date: Jun 16, 2022, 19:20 UTC
Message-ID: <YquCaE+Vw9P/fybU@coredump.intra.peff.net>
In-Reply-To: <xmqqk09gtriq.fsf@gitster.g>

On Thu, Jun 16, 2022 at 11:03:25AM -0700, Junio C Hamano wrote:

Show 8 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > But obviously that's a bad use of the varargs API, I just don't know how
> > we've been getting away with it in practice, sorry about that.
> 
> Exactly.  We three all expressed our surprises on why it "works".
> Nobody offered an explanation, though, which leaves us in suspense
> ;-)

Being the curious sort, I ran it in a debugger. And indeed, "cp" is filled with uninitialized garbage. The reason it works is that the test calls bug() with a format string that does not contain any placeholders. And thus our eventual call to vsnprintf() does not ever look at "cp" at all!

Show 5 quoted lines

> > The fix Peff's got here LGTM. I can (re)submit it with
> > format-patch+send-email after giving it a commit message describing the
> > issue if you'd like, but the change would be the same.
> 
> Yup, I think the code change there looks the most sensible.

I'll wrap it up with a commit message and modify the test to be more thorough.

-Peff