Re: [PATCHv2] attr: convert to new threadsafe API

Stefan Beller [off-list ref] writes:

quoted

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.