Hi Ben, thanks for having a whack at this.

Some tests would be nice.

-static HCERTCHAINENGINE CRYPT_defaultChainEngine;
+/* There are two default chain engines which correspond to HCCE_CURRENT_USER and
+ * HCCE_LOCAL_MACHINE.
+*/
+static HCERTCHAINENGINE CRYPT_defaultChainEngine[2] = { NULL, NULL };

C automatically initializes statics to 0, so the initialization here is unnecessary. I'm also a little uncertain about the use of an array of two objects, I'm not sure that two distinct objects wouldn't be easier to follow, but I'm not religious on this point.

+ if (hChainEngine > HCCE_LOCAL_MACHINE
+ && InterlockedDecrement(&engine->ref) == 0)

I think a function that returns whether an HCERTCHAINENGINE is one of the special ones would make this easier to read, e.g.:

static int is_special_chain_engine(HCERTCHAINENGINE h)

{

return h == HCCE_CURRENT_USER || h == HCCE_LOCAL_MACHINE;

}

then:

+ if (is_special_chain_engine(hChainEngine)
+ && InterlockedDecrement(&engine->ref) == 0)

+static HCERTCHAINENGINE CRYPT_GetDefaultChainEngine(HCERTCHAINENGINE h)
{
- if (!CRYPT_defaultChainEngine)
+ if (!CRYPT_defaultChainEngine[(int)h])

The constant casting is a little awkward. At least introduce a local pointer to the one you intend to modify, so we're not constantly having to re-read that cast.

+ if (hChainEngine <= HCCE_LOCAL_MACHINE)
+ engine = (CertificateChainEngine*)CRYPT_GetDefaultChainEngine(
+ hChainEngine);
if (TRACE_ON(chain))
dump_chain_para(pChainPara);
/* FIXME: what about HCCE_LOCAL_MACHINE? */

See my earlier suggestion on is_special_chain_engine. Also, what about that comment three lines down? Can't it be removed?

-void default_chain_engine_free(void)
+void default_chain_engine_free(HCERTCHAINENGINE h)
{
- CertFreeCertificateChainEngine(CRYPT_defaultChainEngine);
+ CertFreeCertificateChainEngine(CRYPT_defaultChainEngine[(int)h]);

The function default_chain_engine_free is a thin wrapper around CryptFreeCertificateChainEngine; its intended use is to clear memory at shutdown. Rather than shift the responsibility of knowing which engines to free to the caller, just have it free the two engines itself.

Thanks,

--Juan