Re: [PATCH] read-cache.c: reduce unnecessary cache entry name copying

René Scharfe [off-list ref] writes:

quoted

diff --git a/read-cache.c b/read-cache.c
index 96ce489c7c5..e61af3a3d4d 100644
--- a/read-cache.c
+++ b/read-cache.c

@@ -1944,8 +1944,6 @@ static struct cache_entry *create_from_disk(struct mem_pool *ce_mem_pool,
 	ce->ce_namelen = len;
 	ce->index = 0;
 	oidread(&ce->oid, ondisk->data);
-	memcpy(ce->name, name, len);
-	ce->name[len] = '\0';

This removal looks  correct to me.  The extra copying was added by
575fa8a3ed (read-cache: read data in a hash-independent way, 2019-02-19)
but its commit message doesn't mention it.

I agree with the analysis.  When we are prefix-compressing the
entry, name[] may be much shorter than len, so this memcpy() may
potentially be reading beyond the end of the on-disk buffer, so the
original code is not just redundant, but it may simply be wrong.

Thanks, both.  Will queue.

quoted

 	if (expand_name_field) {
 		if (copy_len)

Some more context lines would help to see the redundancy; here they are:

	if (expand_name_field) {
		if (copy_len)
			memcpy(ce->name, previous_ce->name, copy_len);
		memcpy(ce->name + copy_len, name, len + 1 - copy_len);
		*ent_size = (name - ((char *)ondisk)) + len + 1 - copy_len;
	} else {
		memcpy(ce->name, name, len + 1);
		*ent_size = ondisk_ce_size(ce);
	}

quoted

base-commit: ab336e8f1c8009c8b1aab8deb592148e69217085