Re: [PATCH v2 2/7] string-list: align string_list_split() with its _in_place() counterpart

[Junio C Hamano <gitster@pobox.com>]

shejialuo <shejialuo@gmail.com> writes:

> On Thu, Jul 31, 2025 at 03:46:01PM -0700, Junio C Hamano wrote:
>> diff --git a/setup.c b/setup.c
>> index 6f52dab64c..b9f5eb8b51 100644
>> --- a/setup.c
>> +++ b/setup.c
>> @@ -1460,8 +1460,9 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
>>  
>>  	if (env_ceiling_dirs) {
>>  		int empty_entry_found = 0;
>> +		static const char path_sep[] = { PATH_SEP, '\0' };
>>  
>
> I am a little confused why we need to use `static`? Would this function
> be called many times?

I actually am confused why you would want anything other than static
here.  Writing this way would allow the compiler to realize that the
array can be prepared at compile time, without need to do anything
at runtime.  If you made it non static, the runtime code would
allocate two bytes worth of memory on stack, and stuff these two
byte values there, each time this block is entered, which would be
at least once.

> And I have a design question: by using "PATH_SEP", we need to convert
> this character to be string. Should we create a new variable named
> "PATH_SEP_STR" or whatever to do that?

Sorry, but I do not understand the question.  You want to see
something like

	#define PATH_SEP_STR "/"

you mean?  I do not offhand see why anybody would want to do so.

>> -		string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
>> +		string_list_split(&ceiling_dirs, env_ceiling_dirs, path_sep, -1);
>>  		filter_string_list(&ceiling_dirs, 0,
>>  				   canonicalize_ceiling_entry, &empty_entry_found);
>>  		ceil_offset = longest_ancestor_length(dir->buf, &ceiling_dirs);
>
> Thanks,
> Jialuo