Re: [PATCH v2 1/1] read-cache.c: optimize reading index format v4

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 4, 2018, 18:58 UTC
Message-ID: <xmqqk1o19jj8.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180902131933.27484-2-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 11 quoted lines

> +static struct cache_entry *create_from_disk(struct index_state *istate,
>  					    struct ondisk_cache_entry *ondisk,
>  					    unsigned long *ent_size,
> -					    struct strbuf *previous_name)
> +					    const struct cache_entry *previous_ce)
>  {
>  	struct cache_entry *ce;
>  	size_t len;
>  	const char *name;
>  	unsigned int flags;
> +	size_t copy_len;

We should not have to, but let's initialize it to 0 here, because ...

Show 17 quoted lines

> +	if (expand_name_field) {
> +...
> +		copy_len = previous_len - strip_len;
> +		name = (const char *)cp;
> +	}
> +
> +	if (len == CE_NAMEMASK) {
> +		len = strlen(name);
> +		if (expand_name_field)
> +			len += copy_len;
> ...
> +	}
> +	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;

I am seeing a compiler getting confused, thinking that copy_len could be used before getting assigned.

Humans can see that reference to copy_len are made only inside "if (expand_name_field)", so we shouldn't have to.