On Tue, May 3, 2016 at 5:36 PM, Junio C Hamano <[email protected]> wrote:
> Christian Couder <[email protected]> writes:
>
>> +void set_index_file(char *index_file)
>> +{
>> + git_index_file = index_file;
>> +}
>
> What's the rationale for this change, and more importantly, the
> ownership rule for the string? When you call this function, does
> the caller still own that piece of memory? When you call this
> twice, where does the old value go and who is responsible for
> taking care of not leaking it?
>
> Cannot guess any of the above with no proposed log message (that
> comment applies most of your patches in this series).
Yeah, I agree that I could have been more verbose on this one, and
some other ones too.
The reason for this is that run_apply() in builtin/am.c has a "const
char *index_file" argument.
The current version of run_apply() does:
if (index_file)
argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s",
index_file);
to pass that parameter to the `git apply` process that it launches.
I couldn't find a good way other than doing:
if (index_file) {
save_index_file = get_index_file();
set_index_file((char *)index_file);
}
/* Call libified apply functions */
...
if (index_file)
set_index_file(save_index_file);
to do the equivalent of what was done previously.
So I guess I could have written something like the following in the
commit message of the commit that introduces set_index_file():
Introduce set_index_file() to be able to temporarily change the index file.
It should be used like:
/* Save current index file */
old_index_file = get_index_file();
set_index_file((char *)tmp_index_file);
/* Do stuff that will use tmp_index_file as the index file */
...
/* When finished reset the index file */
set_index_file(old_index_file);
Maybe I could also add a comment just before the function.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
