Stefan Beller <sbel...@google.com> writes:
>> But many callers do not follow that; rather they do
>>
>> loop to iterate over paths {
>> call a helper func to learn attr X for path
>> use the value of attr X
>> }
>>
>> using a callchain that embeds a helper function deep inside, and
>> "check" is kept in the helper, check-attr function is called from
>> there, and "result" is not passed from the caller to the helper
>> (obviously, because it does not exist in the current API). See the
>> callchain that leads down to convert.c::convert_attrs() for a
>> typical example. When converted to the new API, it needs to have a
>> new "result" structure every time it is called, and cannot reuse the
>> one that was used in its previous call.
>
> Why would that be? i.e. I do not understand the reasoning/motivation
> as well as what you propose to change here.
The leaf function may look like
check_eol(const char *path)
{
static struct git_attr_check *check;
initl(&check, "eol", NULL);
git_check_attr(&check, path, result);
return nth_element_in_result(result, 0);
}
and we want "result" to be thread-ready.
A naive and undesired way to put it on stack is like so:
const char *check_eol(const char *path)
{
static struct git_attr_check *check;
struct git_attr_result result = RESULT_INIT;
const char *eol;
initl(&check, "eol", NULL);
init_result(&check, &result);
git_check_attr(&check, path, &result);
eol = nth_element_in_result(&result, 0);
clear_result(&result);
return eol;
}
where your "struct git_attr_result" has a fixed size, and the actual
result array is allocated via ALLOC_GROW() etc. That's overhead
that we do not want. Instead can't we do this?
const char *check_eol(const char *path)
{
static struct git_attr_check *check;
/* we know we only want one */
struct git_attr_result result[1];
const char *eol;
initl(&check, "eol", NULL);
git_check_attr(&check, path, result);
return result[0];
}
That way, we won't be doing ALLOC_GROW() in init_result() or free in
clear_result().
If you use a structure that has pointer to an array (i.e. the "naive
and undesired way" above), you cannot amortize the alloc/free by
making result "static" if you want to be thread-ready.
