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

["René Scharfe" <l.s.r@web.de>]

Am 08.01.2023 um 08:28 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>
>> Use __builtin_types_compatible_p to perform a full type check if
>> possible.  Otherwise fall back to the old size comparison, but add a
>> non-evaluated assignment to catch more type mismatches.  It doesn't flag
>> copies between arrays with different signedness, but that's as close to
>> a full type check as it gets without the builtin, as far as I can see.
>
> This seems to unfortunately break builds for compat/mingw.c
>
> cf. https://github.com/git/git/actions/runs/3865788736/jobs/6589504628#step:4:374
>
>    1848 |                 COPY_ARRAY(&argv2[1], &argv[1], argc);
>
> where the two arrays are "char *const *argv" in the parameter list, and
> a local variable
>
> #ifndef _MSC_VER
> 		const
> #endif
> 		char **argv2;
>
> It seems that (const char **) and (char **) are compatible but the
> pointers themselves being const breaks the type compatibility?
> Perhaps the latter should be "(optionally const) char *const *argv2"?

We compare the types of the elements, so effectively we do this:

   __builtin_types_compatible_p(__typeof__(const char *),  __typeof__(char *))

... which returns 0.

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. :-|

--- >8 ---
Subject: [PATCH 1.5/4] mingw: make argv2 in try_shell_exec() non-const

Prepare for a stricter type check in COPY_ARRAY by removing the const
qualifier of argv2, like we already do to placate Visual Studio.  We
have to add it back using explicit casts when actually using the
variable, unfortunately, because GCC (rightly) refuses to add it
implicitly.  Similar casts are already used in mingw_execv().

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 compat/mingw.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index d614f156df..e131eb9b07 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1839,16 +1839,13 @@ static int try_shell_exec(const char *cmd, char *const *argv)
 	if (prog) {
 		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