Re: [PATCH v2 2/4] do full type check in BARF_UNLESS_COPYABLE

[Jeff Hostetler <git@jeffhostetler.com>]

On 1/8/23 11:27 PM, Junio C Hamano wrote:
> René Scharfe <l.s.r@web.de> writes:
> 
>> We compare the types of the elements, so effectively we do this:
>>
>>     __builtin_types_compatible_p(__typeof__(const char *),  __typeof__(char *))
>>
>> ... which returns 0.
> 
> True.  I wonder if (const const char *) and (const char *) are
> deemed compatible?  Even if so, probably we cannot write
> 
> 	__builtin_types_compatible_p(const __typeof__(*(dst)),
> 				     const __typeof__(*(src)))
> 
> so that line of thoguht would lead nowhere X-<.
> 
>> We can remove the const like we already do for Visual Studio.  But
>> then we have to add two casts when passing on argv2, like in
>> mingw_execv(), because adding a const to a pointer of a pointer
>> must be done explicitly in C (even though Visual Studio seems to
>> do it implicitly without complaining).  Feels a bit silly. :-|
> 
> Indeed.  Let's see what folks, whom "git blame" tells us to be area
> experts around here, think.  The "if _MSC, add const" was added in
> 12fb9bd8 (msvc: mark a variable as non-const, 2019-06-19) by JeffH,
> and try_shell_exec() function itself came from f1a4dfb8 (Windows:
> Wrap execve so that shell scripts can be invoked., 2007-12-04),
> added by J6t.

I added the ifNdef.  The existing code already had the const
that MSVC didn't like.  I don't remember why I didn't just remove
the const completely.  If I had to guess I'd say that dropping
probably caused a small cascade of caller/callee type changes
and that would have distracted from the focus of my patch series
at the time.

Personally, I think we should just remove the const from all
versions.

Thanks,
Jeff


...
>>   		int exec_id;
>>   		int argc = 0;
>> -#ifndef _MSC_VER
>> -		const
>> -#endif
>>   		char **argv2;
>>   		while (argv[argc]) argc++;
>>   		ALLOC_ARRAY(argv2, argc + 1);
>>   		argv2[0] = (char *)cmd;	/* full path to the script file */
>>   		COPY_ARRAY(&argv2[1], &argv[1], argc);
>> -		exec_id = trace2_exec(prog, argv2);
>> -		pid = mingw_spawnv(prog, argv2, 1);
>> +		exec_id = trace2_exec(prog, (const char **)argv2);
>> +		pid = mingw_spawnv(prog, (const char **)argv2, 1);
>>   		if (pid >= 0) {
>>   			int status;
>>   			if (waitpid(pid, &status, 0) < 0)
>> --
>> 2.38.1.windows.1