On 2/24/21 4:05 PM, Jeff King wrote:
On Wed, Feb 24, 2021 at 12:08:42PM -0800, Junio C Hamano wrote:
Use FLEX_ALLOC_STR() to allocate the `struct untracked_cache_dir`
for the root directory. Get rid of unsafe code that might fail to
initialize the `name` field (if FLEX_ARRAY is not 1). This will
make it clear that we intend to have a structure with an empty
string following it.
[...]
The problematic code was introduced in 2015, a year before these
FLEX_ALLOC_*() helpers were introduced. The result is of course
correct and much easier to read, as the necessary "ask for a region
of calloc'ed memory with an additional byte for terminating NUL
beyond strlen()" is hidden in the helper.
When I added the FLEX_ALLOC_* helpers, I audited for existing callers to
convert. But I did so by looking for places where we were doing manual
size computations. The bug here was that it was not doing any
computation at all (when it need to be doing "+1"). So that's my guess
why it got overlooked (which is not super important, but may give a hint
about how to look for similar bugs).
There's another call to xmalloc() at [1] that does the st_add3()
thing that I didn't change. It was correctly computing the size
so I didn't want to change it for no reason. That and the 2 memcpy()s
made it feel like we should have a different FLEX_ macro for this
case -- more of a FLEX_DUP... or something. But I didn't want to
distract from my bug fix here.
[1] https://github.com/git/git/blob/master/dir.c#L3290
Jeff
