Re: [PATCH] msi: Print error number when GetProcAddress fails

[Hans Leidekker]

On Wed, 2017-08-16 at 17:20 +0100, Carlos Palminha wrote:
> diff --git a/dlls/msi/custom.c b/dlls/msi/custom.c
> index 922927e8903..cfdfe852efb 100644
> --- a/dlls/msi/custom.c
> +++ b/dlls/msi/custom.c
> @@ -608,7 +608,7 @@ static DWORD ACTION_CallDllFunction( const GUID *guid )
>              ERR("failed to create handle for %p\n", remote_package );
>      }
>      else
> -
>    ERR("GetProcAddress(%s) failed\n", debugstr_w( function ) );
> -
>    ERR("GetProcAddress(%s) failed: %d\n", debugstr_w( function ), GetLastError() );
>
> FreeLibrary(hModule);

This doesn't appear to add any useful information. Our implementation of
GetProcAddress sets either ERROR_PROC_NOT_FOUND or ERROR_MOD_NOT_FOUND and
LoadLibrary must have succeeded before reaching this code.

[PATCH] msi: Print error number when GetProcAddress fails

[Carlos Palminha]

On 17-08-2017 10:24, Hans Leidekker wrote:
> On Wed, 2017-08-16 at 17:20 +0100, Carlos Palminha wrote:
> > diff --git a/dlls/msi/custom.c b/dlls/msi/custom.c
> > index 922927e8903..cfdfe852efb 100644
> > --- a/dlls/msi/custom.c
> > +++ b/dlls/msi/custom.c
> > @@ -608,7 +608,7 @@ static DWORD ACTION_CallDllFunction( const GUID *guid )
> >              ERR("failed to create handle for %p\n", remote_package );
> >      }
> >      else
> > -
> >    ERR("GetProcAddress(%s) failed\n", debugstr_w( function ) );
> > -
> >    ERR("GetProcAddress(%s) failed: %d\n", debugstr_w( function ), GetLastError() );
> >
> > FreeLibrary(hModule);
>
> This doesn't appear to add any useful information. Our implementation of
> GetProcAddress sets either ERROR_PROC_NOT_FOUND or ERROR_MOD_NOT_FOUND and
> LoadLibrary must have succeeded before reaching this code.

This is useful exactly for the cases where the library handle is ok but it fails trying to call a function from the DLL.
Printing the the error number is coherent with other cases and bring extra info about what went wrong calling the function.

[PATCH] msi: Print error number when GetProcAddress fails

[Nikolay Sivov]

On 08/17/2017 01:24 PM, Carlos Palminha wrote:
> On 17-08-2017 10:24, Hans Leidekker wrote:
> > On Wed, 2017-08-16 at 17:20 +0100, Carlos Palminha wrote:
> > > diff --git a/dlls/msi/custom.c b/dlls/msi/custom.c
> > > index 922927e8903..cfdfe852efb 100644
> > > --- a/dlls/msi/custom.c
> > > +++ b/dlls/msi/custom.c
> > > @@ -608,7 +608,7 @@ static DWORD ACTION_CallDllFunction( const GUID *guid )
> > >               ERR("failed to create handle for %p\n", remote_package );
> > >       }
> > >       else
> > > -
> > >    ERR("GetProcAddress(%s) failed\n", debugstr_w( function ) );
> > > -
> > >    ERR("GetProcAddress(%s) failed: %d\n", debugstr_w( function ), GetLastError() );
> > >
> > > FreeLibrary(hModule);
> >
> > This doesn't appear to add any useful information. Our implementation of
> > GetProcAddress sets either ERROR_PROC_NOT_FOUND or ERROR_MOD_NOT_FOUND and
> > LoadLibrary must have succeeded before reaching this code.
>
> This is useful exactly for the cases where the library handle is ok but it fails trying to call a function from the DLL.
> Printing the the error number is coherent with other cases and bring extra info about what went wrong calling the function.

Missing is an error condition on its own. The point is it's either 
missing or present, extra error code won't tell anything new.