Elijah Newren <newren@xxxxxxxxx> writes:
>> /* Pre-allocate some space in buf */
>> extra = hash_size + 8; /* 8: 6 for mode, 1 for space, 1 for NUL char */
>>
>> base-commit: 89b43f80a514aee58b662ad606e6352e03eaeee4
>> --
>> gitgitgadget
>
> Otherwise, this patch looks good to me; thanks!
By the way, I noticed the post-context comment and got curious.
/* Pre-allocate some space in buf */
extra = hash_size + 8; /* 8: 6 for mode, 1 for space, 1 for NUL char */
for (i = 0; i < nr; i++) {
maxlen += strlen(versions->items[offset+i].string) + extra;
}
strbuf_grow(&buf, maxlen);
Because "6 for mode" is wrong if it means "%06o", but the format
used in the code is "%o" so there is no correctness issues in the
code (Phew). This "grow" is to avoid having to repeatedly
reallocate "nr" times in the loop that comes, but
(1) s/6 for mode/up to 6 for mode/ to make it less misleading.
(2) does this preallocation really help performance?
(3) it is really disturbing to find a custom tree-writing code that
is exercised only by ort here. Tree-writing has been one major
source of broken objects in various reimplementation of Git---
all known ones made a mistake sometime in the past---and I'd
strongly prefer to see a single helper that formats a tree
object, given a set of <mode, object name, path> in the longer
code health.
