Re: crypt32: Support HCCE_LOCAL_MACHINE.

[Juan Lang]

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