On Sun, Oct 6, 2024 at 11:45 PM shejialuo [off-list ref] wrote:

Actually, I somehow understand why in the original code, it will use
"xstrdup_or_null" to initialize the "backlink". Because in
"read_gitfile_gently", we will use a static "struct strbuf" as the
buffer.

I guess the intention of the original code is that if we call again
"read_gitfile_gently" in this function or we have another thread which
calls this function, the content of the buffer will be changed. So we
explicitly use "xstrdup_or_null" to create a copy here to avoid this.

That's correct. The code is being defensive in case the static strbuf
inside read_gitfile_gently() gets overwritten.

But I wonder whether we really need to consider above problem?

It's easier to reason about the code in this function if we don't have
to worry about the underlying static strbuf. Also, this is not a hot
path so the extra allocation and string copy is not a concern. As
such, it makes sense for the code to remain robust (by creating the
copy) rather than becoming potentially more fragile by eliminating the
string copy.