René Scharfe <l.s.r@xxxxxx> writes:
>> 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.
>>
>> 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);
> }
>
>>
>> base-commit: ab336e8f1c8009c8b1aab8deb592148e69217085
