Re: ADVAPI32: preserve the last error value set by the CSP in CryptAcquireContext

[Robert Shearman]

Mike McCormack wrote:
> ChangeLog:
> * preserve the last error value set by the CSP in CryptAcquireContext
>
> ------------------------------------------------------------------------
>
> Index: dlls/advapi32/crypt.c
> ===================================================================
> RCS file: /home/wine/wine/dlls/advapi32/crypt.c,v
> retrieving revision 1.51
> diff -u -r1.51 crypt.c
> --- dlls/advapi32/crypt.c	30 Aug 2004 23:57:18 -0000	1.51
> +++ dlls/advapi32/crypt.c	4 Sep 2004 01:09:44 -0000
> @@ -476,6 +476,8 @@
>   }
>   /* FALLTHROUGH TO ERROR IF FALSE - CSP internal error! */
> error:
> +	/* must preserve the last error returned by the CSP here */
> +	r = GetLastError();
>   if (pProv)
>   {
>       FreeLibrary(pProv->hModule);
> @@ -486,6 +488,7 @@
>   CRYPT_Free(provname);
>   CRYPT_Free(temp);
>   CRYPT_Free(imagepath);
> +	SetLastError( r );
>   return FALSE;
> }

This seems wrong. Which function is calling SetLastError? I believe 
there was a problem a while ago with freeing bad pointers. I can't 
remember whether a fix went in for that.

Rob

ADVAPI32: preserve the last error value set by the CSP in CryptAcquireContext

[Mike McCormack]

Robert Shearman wrote:
> > error:
> > +    /* must preserve the last error returned by the CSP here */
> > +    r = GetLastError();
> >     if (pProv)
> >     {
> >         FreeLibrary(pProv->hModule);
> > @@ -486,6 +488,7 @@
> >     CRYPT_Free(provname);
> >     CRYPT_Free(temp);
> >     CRYPT_Free(imagepath);
> > +    SetLastError( r );
> >     return FALSE;
> > }
>
> This seems wrong. Which function is calling SetLastError? I believe 
> there was a problem a while ago with freeing bad pointers. I can't 
> remember whether a fix went in for that.
>
> Rob

Yeah, well, something there is calling SetLastError(). We can muck round 
trying to fix it by finding which pointer is bad, but the problem can 
easily rear it's head again unless there's a comment or specific code in 
there to point out the problem.

A better fix would be to convert it to using RtlHeapAlloc() and 
RtlHeapFree() so that the last error isn't touched, and perhaps use 
LdrFreeLibrary too.

Mike