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

Elijah Newren <newren@gmail.com> writes:

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.