Junio C Hamano <gitster@xxxxxxxxx> writes:
> Glen Choo <chooglen@xxxxxxxxxx> writes:
>
>> + char *path;
>> /* The submodule commits that have changed in the rev walk. */
>> struct oid_array new_commits;
>> };
>> @@ -818,6 +828,7 @@ struct changed_submodule_data {
>> static void changed_submodule_data_clear(struct changed_submodule_data *cs_data)
>> {
>> oid_array_clear(&cs_data->new_commits);
>> + free(cs_data->path);
>
> OK.
>
>> }
>>
>> static void collect_changed_submodules_cb(struct diff_queue_struct *q,
>> @@ -865,6 +876,8 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,
>> if (!item->util)
>> item->util = xcalloc(1, sizeof(struct changed_submodule_data));
>> cs_data = item->util;
>> + cs_data->super_oid = commit_oid;
>> + cs_data->path = xstrdup(p->two->path);
>
> Iffy. If item->util were populated already, wouldn't cs_data
> already have its .path member pointing at an allocated piece of
> memory? Can we safely free it before assigning a new value, or does
> somebody else still have a copy of .path and we cannot free it?
Great catch! This is a silly mistake, it looks like this because I
copied the pattern that we used to _append_ new commit oids, but
.super_oid and .path aren't appended, they're replaced.
But we don't even need to replace .super_oid and .path, we can use the
first values we encounter and ignore subsequent ones.
