Re: [PATCH v2] merge-ort: only do pointer arithmetic for non-empty lists

Elijah Newren [off-list ref] writes:

quoted

        /* 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.