Re: [PATCH cb/pedantic-build-for-developers] lazyload.h: fix warnings about mismatching function pointer types

[Carlo Arenas <carenas@gmail.com>]

On Wed, Sep 22, 2021 at 2:21 PM Johannes Sixt <j6t@kdbg.org> wrote:
>
> Am 22.09.21 um 22:16 schrieb Carlo Arenas:
> > On Wed, Sep 22, 2021 at 12:56 PM Johannes Sixt <j6t@kdbg.org> wrote:
> >>
> >> Here, GCC warns about every use of the INIT_PROC_ADDR macro, for example:
> >>
> >> In file included from compat/mingw.c:8:
> >> compat/mingw.c: In function 'mingw_strftime':
> >> compat/win32/lazyload.h:38:12: warning: assignment to
> >>    'size_t (*)(char *, size_t,  const char *, const struct tm *)'
> >>    {aka 'long long unsigned int (*)(char *, long long unsigned int,
> >>       const char *, const struct tm *)'} from incompatible pointer type
> >>    'FARPROC' {aka 'long long int (*)()'} [-Wincompatible-pointer-types]
> >>    38 |  (function = get_proc_addr(&proc_addr_##function))
> >>       |            ^
> >> compat/mingw.c:1014:6: note: in expansion of macro 'INIT_PROC_ADDR'
> >>  1014 |  if (INIT_PROC_ADDR(strftime))
> >>       |      ^~~~~~~~~~~~~~
> >
> > did you have CFLAGS adding -Wincompatible-pointer-types explicitly?
>
> I don't know of the top of my head (am not at that Windows box right
> now). I am fairly certain that I do not have DEVELOPER set.

the config.mak.uname for MinGW sets that for you.

> > This is the reason why the code that got merged to master had -Wno
> > for this case.
> >
> >> (message wrapper for convenience). Insert a cast to keep the compiler
> >> happy. A cast is fine in these cases because they are generic function
> >> pointer values that have been looked up in a DLL.
> >
> > I have a more complete "fix" which I got stuck testing GGG[1]; you are likely
> > going to also hit -Wcast-function-type otherwise.

Sadly, as predicted your fix, broke the build [1], so submitted[2] and
adaptation
of the original fix on top of yours with the rest of the code that
will be needed
so it can be added in top of js/win-lazyload-buildfix and merged into "seen" to
fix that.

> I think that the correct solution is that get_proc_addr() returns void*,
> not FARPROC. Then either no cast is needed (because void* can be
> converted to function pointer type implicitly) or a cast is needed and
> that is then not between incompatible function pointer types and should
> not trigger -Wcast-function-type, theoretically.

The reasons behind why won't work are fairly academic IMHO, but there is
an obvious clash between POSIX and C specs here and "pedantic" just
reflects that.

Note that the return type for GetProcAddress[3] is FARPROC, and the C
standard says that any function pointer can be assigned to any other so
your code should work, but gcc has no way to know that FARPROC is not
the "real" function type and therefore warns that you might crash if using it.

Carlo

[1] https://github.com/git/git/runs/3680695336
[2] https://lore.kernel.org/git/20210923060306.21073-1-carenas@gmail.com/
[3] https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-getprocaddress