Re: [PATCH v2 2/4] do full type check in BARF_UNLESS_COPYABLE
- From
René Scharfe <l.s.r@web.de>
- Date
- Jan 8, 2023, 10:10 UTC
- Message-ID
- <3e04e283-cad0-7be4-d85c-65d0a52289e2@web.de>
- In-Reply-To
- <xmqqy1qdxz67.fsf@gitster.g>
Am 08.01.2023 um 08:28 schrieb Junio C Hamano:
Show 25 quoted lines
> 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