| Message ID | 20210926100512.58924-3-carenas@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | js/win-lazyload-buildfix | expand |
Carlo Marcelo Arenas Belón <carenas@gmail.com> writes: > gcc will helpfully raise a -Wcast-function-type warning when casting > between functions that might have incompatible return types > (ex: GetUserNameExW returns bool which is only half the size of the > return type from FARPROC which is long long), so create a new type that > could be used as a completely generic function pointer and cast through > it instead. > > Because of the way the function declaration was done in the previous > patch the order of variables that use it had to be adjusted so that > it is the last variable declared, as well. Is it clear to everybody what this paragraph is referring to? It is not, at least to me. > > Additionaly remove the -Wno-incompatible-pointer-types temporary > flag added in 27e0c3c (win32: allow building with pedantic mode > enabled, 2021-09-03), as it will be no longer needed. > > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> > --- > v3 > - removes unnecessary variable moving after fixup in previous patch Thanks.
On Mon, Sep 27, 2021 at 9:35 AM Junio C Hamano <gitster@pobox.com> wrote: > > Carlo Marcelo Arenas Belón <carenas@gmail.com> writes: > > > Because of the way the function declaration was done in the previous > > patch the order of variables that use it had to be adjusted so that > > it is the last variable declared, as well. > > Is it clear to everybody what this paragraph is referring to? It is > not, at least to me. It is not, and it is no longer needed after the fixup was applied to the previous patch. Do you want me to send another series removing it or can be done while applying? It was slightly better explained in the fixup[1] commit message. Because of the double ';', When the macro was used to declare a function variable and it was not the last variable declared, then it will trigger -Wdeclaration-after-statement. Carlo [1] https://lore.kernel.org/git/3f963bba-3197-8c52-9828-6d78ef1d25b1@kdbg.org/
Carlo Arenas <carenas@gmail.com> writes: > On Mon, Sep 27, 2021 at 9:35 AM Junio C Hamano <gitster@pobox.com> wrote: >> >> Carlo Marcelo Arenas Belón <carenas@gmail.com> writes: >> >> > Because of the way the function declaration was done in the previous >> > patch the order of variables that use it had to be adjusted so that >> > it is the last variable declared, as well. >> >> Is it clear to everybody what this paragraph is referring to? It is >> not, at least to me. > > It is not, and it is no longer needed after the fixup was applied to > the previous > patch. Ah, yes, the previous one's definition is quite awkward because you had to omit the terminating ';' to use it. I'll drop the paragraph with "commit --amend". THanks.
diff --git a/compat/win32/lazyload.h b/compat/win32/lazyload.h index 121ee24ed2..2b3637135f 100644 --- a/compat/win32/lazyload.h +++ b/compat/win32/lazyload.h @@ -15,10 +15,12 @@ * source, target); */ +typedef void (*FARVOIDPROC)(void); + struct proc_addr { const char *const dll; const char *const function; - FARPROC pfunction; + FARVOIDPROC pfunction; unsigned initialized : 1; }; @@ -38,7 +40,7 @@ struct proc_addr { #define INIT_PROC_ADDR(function) \ (function = (proc_type_##function)get_proc_addr(&proc_addr_##function)) -static inline FARPROC get_proc_addr(struct proc_addr *proc) +static inline FARVOIDPROC get_proc_addr(struct proc_addr *proc) { /* only do this once */ if (!proc->initialized) { @@ -47,7 +49,8 @@ static inline FARPROC get_proc_addr(struct proc_addr *proc) hnd = LoadLibraryExA(proc->dll, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); if (hnd) - proc->pfunction = GetProcAddress(hnd, proc->function); + proc->pfunction = (FARVOIDPROC)GetProcAddress(hnd, + proc->function); } /* set ENOSYS if DLL or function was not found */ if (!proc->pfunction) diff --git a/config.mak.dev b/config.mak.dev index c080ac0231..cdf043c52b 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -12,7 +12,6 @@ DEVELOPER_CFLAGS += -pedantic DEVELOPER_CFLAGS += -Wpedantic ifneq ($(filter gcc5,$(COMPILER_FEATURES)),) DEVELOPER_CFLAGS += -Wno-pedantic-ms-format -DEVELOPER_CFLAGS += -Wno-incompatible-pointer-types endif endif DEVELOPER_CFLAGS += -Wdeclaration-after-statement
gcc will helpfully raise a -Wcast-function-type warning when casting between functions that might have incompatible return types (ex: GetUserNameExW returns bool which is only half the size of the return type from FARPROC which is long long), so create a new type that could be used as a completely generic function pointer and cast through it instead. Because of the way the function declaration was done in the previous patch the order of variables that use it had to be adjusted so that it is the last variable declared, as well. Additionaly remove the -Wno-incompatible-pointer-types temporary flag added in 27e0c3c (win32: allow building with pedantic mode enabled, 2021-09-03), as it will be no longer needed. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> --- v3 - removes unnecessary variable moving after fixup in previous patch compat/win32/lazyload.h | 9 ++++++--- config.mak.dev | 1 - 2 files changed, 6 insertions(+), 4 deletions(-)