fusion: Use proper function pointer

Michael Karcher wine at mkarcher.dialup.fu-berlin.de
Sun Aug 3 07:18:24 CDT 2008


Am Samstag, den 02.08.2008, 17:51 -0500 schrieb James Hawkins:
> > -static HRESULT (WINAPI *pGetCORVersion)(LPWSTR pbuffer, DWORD cchBuffer,
> > -                                        DWORD *dwLength);
> > -
> 
> There's nothing wrong with this.  We use this type of function pointer
> all over the code base.

You don't quote the interesting part:
> > -    pGetCORVersion = (void *)GetProcAddress(hmscoree, "GetCORVersion");
> > +    pGetCORVersion = (PFNGETCORVERSION)GetProcAddress(hmscoree, "GetCORVersion");

This *is* wrong, according to the C standard. You may not cast a
function pointer to a data pointer (like void*). There might be
precedence in Wine to ignore this rule, but the patch seems appropriate
at this point, as GetProcAddress does, in fact, return a
pointer-to-function (FARPROC, which is typedefed in windef.h to a
function pointer), which gets assigned to a function pointer. The
(void*) cast is an IMHO misguided attempt to suppress the warning of
casting between to function pointers of different types. As there is no
void* like generic pointer for function pointers, the only sensible way
to suppress the warning without causing undefined behaviour really is
casting the result of GetProcAddress to the type of the destination
pointer. As the casts to function pointer types look utterly
incomprehensible in C, it seems correct to introduce a typedef, like the
patch does.

Another point of the patch is that is makes a static global variable a
local function variable. I don't take a position on that change,
although on the first glance it seems reasonable, as the global variable
is initialized on each entry into get_corversion. If the address of
GetCORVersion should be cached between invocations, the GetProcAddress
and LoadLibraryA calls should probably be guarded by pGetCORVersion !=
NULL.

Regards,
  Michael Karcher




More information about the wine-devel mailing list