David Turner <dturner@xxxxxxxxxxxxxxxx> writes:
> From: Keith McGuigan <kmcguigan@xxxxxxxxxxx>
>
> During merges, we would previously free entries that we no longer need
> in the destination index. But those entries might also be stored in
> the dir_entry cache, and when a later call to add_to_index found them,
> they would be used after being freed.
>
> To prevent this, add a ref count for struct cache_entry. Whenever
> a cache entry is added to a data structure, the ref count is incremented;
> when it is removed from the data structure, it is decremented. When
> it hits zero, the cache_entry is freed.
>
> Signed-off-by: Keith McGuigan <kmcguigan@xxxxxxxxxxx>
> ---
I'll forge your "messenger's sign-off" here ;-)
> diff --git a/unpack-trees.c b/unpack-trees.c
> index f932e80..1a0a637 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -606,8 +606,10 @@ static int unpack_nondirectories(int n, unsigned long mask,
> o);
> for (i = 0; i < n; i++) {
> struct cache_entry *ce = src[i + o->merge];
> - if (ce != o->df_conflict_entry)
> - free(ce);
> + if (ce != o->df_conflict_entry) {
> + drop_ce_ref(ce);
> + src[i + o->merge] = NULL;
> + }
This one smelled iffy. I think it is safe because the caller does
not look at src[] other than src[0] after this function returns, and
this setting to NULL happens only when o->merge is set to 1, so I do
not think this is buggy, but at the same time I do not think setting
to NULL is necessary.
Other than that, looks nice. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html