November 2009 - Wine-Commits

Juan Lang : crypt32: Eliminate a double free in the tests.
by Alexandre Julliard Nov. 4, 2009

Nov. 4, 2009

Module: wine Branch: master Commit: f8376b91da0b4fb973dec29c603a06c405d8d1d8 URL: http://source.winehq.org/git/wine.git/?a=commit;h=f8376b91da0b4fb973dec29c6… Author: Juan Lang <juan.lang(a)gmail.com> Date: Tue Nov 3 17:22:10 2009 -0800 crypt32: Eliminate a double free in the tests. --- dlls/crypt32/tests/store.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/dlls/crypt32/tests/store.c b/dlls/crypt32/tests/store.c index 419d2dd..5c019fa 100644 --- a/dlls/crypt32/tests/store.c +++ b/dlls/crypt32/tests/store.c @@ -2096,7 +2096,6 @@ static void test_I_UpdateStore(void) certs = countCertsInStore(store1); ok(certs == 0, "Expected 0 certs, got %d\n", certs); - CertFreeCertificateContext(cert); CertCloseStore(store1, 0); CertCloseStore(store2, 0); }

1 0

Juan Lang : crypt32: Don't delete a context when removing it from a list.
by Alexandre Julliard Nov. 4, 2009

Nov. 4, 2009

Module: wine Branch: master Commit: df39bbba4f0317aeed8860761067407af11a5c07 URL: http://source.winehq.org/git/wine.git/?a=commit;h=df39bbba4f0317aeed8860761… Author: Juan Lang <juan.lang(a)gmail.com> Date: Fri Oct 30 18:19:39 2009 -0700 crypt32: Don't delete a context when removing it from a list. --- dlls/crypt32/context.c | 1 - dlls/crypt32/tests/store.c | 1 - 2 files changed, 0 insertions(+), 2 deletions(-) diff --git a/dlls/crypt32/context.c b/dlls/crypt32/context.c index e0c1932..73e1e0e 100644 --- a/dlls/crypt32/context.c +++ b/dlls/crypt32/context.c @@ -321,7 +321,6 @@ void ContextList_Delete(struct ContextList *list, void *context) list_remove(entry); LeaveCriticalSection(&list->cs); list_init(entry); - list->contextInterface->free(context); } static void ContextList_Empty(struct ContextList *list) diff --git a/dlls/crypt32/tests/store.c b/dlls/crypt32/tests/store.c index 1f6609c..419d2dd 100644 --- a/dlls/crypt32/tests/store.c +++ b/dlls/crypt32/tests/store.c @@ -237,7 +237,6 @@ static void testMemStore(void) GetLastError()); /* try deleting a copy */ ret = CertDeleteCertificateFromStore(copy); - todo_wine ok(ret, "CertDeleteCertificateFromStore failed: %08x\n", GetLastError()); /* check that the store is empty */

1 0

Juan Lang : crypt32: Trace whenever a reference count changes, and change default debug channel to quiet the main crypt channel.
by Alexandre Julliard Nov. 4, 2009

Nov. 4, 2009

Module: wine Branch: master Commit: 1f363cd399a9541992f2171c28faf8477d6c096b URL: http://source.winehq.org/git/wine.git/?a=commit;h=1f363cd399a9541992f2171c2… Author: Juan Lang <juan.lang(a)gmail.com> Date: Tue Nov 3 16:44:44 2009 -0800 crypt32: Trace whenever a reference count changes, and change default debug channel to quiet the main crypt channel. --- dlls/crypt32/context.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/dlls/crypt32/context.c b/dlls/crypt32/context.c index 109fcd4..b8ad220 100644 --- a/dlls/crypt32/context.c +++ b/dlls/crypt32/context.c @@ -24,7 +24,7 @@ #include "wine/list.h" #include "crypt32_private.h" -WINE_DEFAULT_DEBUG_CHANNEL(crypt); +WINE_DEFAULT_DEBUG_CHANNEL(context); typedef enum _ContextType { ContextTypeData, @@ -106,6 +106,7 @@ void Context_AddRef(void *context, size_t contextSize) PBASE_CONTEXT baseContext = BASE_CONTEXT_FROM_CONTEXT(context, contextSize); InterlockedIncrement(&baseContext->ref); + TRACE("%p's ref count is %d\n", context, baseContext->ref); if (baseContext->type == ContextTypeLink) { void *linkedContext = Context_GetLinkedContext(context, contextSize); @@ -116,6 +117,7 @@ void Context_AddRef(void *context, size_t contextSize) while (linkedContext && linkedBase->type == ContextTypeLink) { InterlockedIncrement(&linkedBase->ref); + TRACE("%p's ref count is %d\n", linkedContext, linkedBase->ref); linkedContext = Context_GetLinkedContext(linkedContext, contextSize); if (linkedContext) @@ -132,6 +134,7 @@ void Context_AddRef(void *context, size_t contextSize) linkedBase = BASE_CONTEXT_FROM_CONTEXT(linkedContext, contextSize); InterlockedIncrement(&linkedBase->ref); + TRACE("%p's ref count is %d\n", linkedContext, linkedBase->ref); } } }

1 0

Juan Lang : crypt32: When creating a link context, call Context_AddRef to add-ref it so its children get add-ref'd too.
by Alexandre Julliard Nov. 4, 2009

Nov. 4, 2009

Module: wine Branch: master Commit: 52820b9cf80b9fda123b32884643c9f815be853c URL: http://source.winehq.org/git/wine.git/?a=commit;h=52820b9cf80b9fda123b32884… Author: Juan Lang <juan.lang(a)gmail.com> Date: Tue Nov 3 17:05:29 2009 -0800 crypt32: When creating a link context, call Context_AddRef to add-ref it so its children get add-ref'd too. --- dlls/crypt32/context.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/dlls/crypt32/context.c b/dlls/crypt32/context.c index 4d325c1..2f1720a 100644 --- a/dlls/crypt32/context.c +++ b/dlls/crypt32/context.c @@ -94,7 +94,7 @@ void *Context_CreateLinkContext(unsigned int contextSize, void *linked, unsigned linkContext->type = ContextTypeLink; linkContext->linked = linkedBase; if (addRef) - InterlockedIncrement(&linkedBase->ref); + Context_AddRef(linked, contextSize); TRACE("%p's ref count is %d\n", context, linkContext->ref); } TRACE("returning %p\n", context);

1 0

Juan Lang : wintrust: Only close a file in SoftpubCleanup if the WINTRUST_DATA contains a WINTRUST_FILE_INFO .
by Alexandre Julliard Nov. 4, 2009

Nov. 4, 2009

Module: wine Branch: master Commit: f4b359942b263b16f9a4b1a6694a31f83c6774b2 URL: http://source.winehq.org/git/wine.git/?a=commit;h=f4b359942b263b16f9a4b1a66… Author: Juan Lang <juan.lang(a)gmail.com> Date: Tue Nov 3 16:15:32 2009 -0800 wintrust: Only close a file in SoftpubCleanup if the WINTRUST_DATA contains a WINTRUST_FILE_INFO. --- dlls/wintrust/softpub.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/dlls/wintrust/softpub.c b/dlls/wintrust/softpub.c index 1db4b47..47b76b2 100644 --- a/dlls/wintrust/softpub.c +++ b/dlls/wintrust/softpub.c @@ -1078,7 +1078,8 @@ HRESULT WINAPI SoftpubCleanup(CRYPT_PROVIDER_DATA *data) CryptMsgClose(data->hMsg); - if (data->fOpenedFile) + if (data->fOpenedFile && + data->pWintrustData->dwUnionChoice == WTD_CHOICE_FILE) CloseHandle(data->pWintrustData->u.pFile->hFile); return S_OK;

1 0

Jeremy White : advapi32/tests: Only release providers and free keys if we' ve had success creating them.
by Alexandre Julliard Nov. 4, 2009

Nov. 4, 2009

Module: wine Branch: master Commit: 5df2626aaeab15a13364f0c1e660d933b577ffd3 URL: http://source.winehq.org/git/wine.git/?a=commit;h=5df2626aaeab15a13364f0c1e… Author: Jeremy White <jwhite(a)codeweavers.com> Date: Mon Nov 2 16:46:07 2009 -0600 advapi32/tests: Only release providers and free keys if we've had success creating them. --- dlls/advapi32/tests/crypt.c | 185 +++++++++++++++++++++++-------------------- 1 files changed, 99 insertions(+), 86 deletions(-) diff --git a/dlls/advapi32/tests/crypt.c b/dlls/advapi32/tests/crypt.c index 131e605..9eacbd4 100644 --- a/dlls/advapi32/tests/crypt.c +++ b/dlls/advapi32/tests/crypt.c @@ -956,70 +956,78 @@ static void test_rc2_keylen(void) ret = pCryptAcquireContextA(&provider, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT); ok(ret, "CryptAcquireContext error %u\n", GetLastError()); - - key_blob.header.bType = PLAINTEXTKEYBLOB; - key_blob.header.bVersion = CUR_BLOB_VERSION; - key_blob.header.reserved = 0; - key_blob.header.aiKeyAlg = CALG_RC2; - key_blob.key_size = sizeof(key); - memcpy(key_blob.key_data, key, key_length); - - /* Importing a 16-byte key works with the default provider. */ - SetLastError(0xdeadbeef); - ret = pCryptImportKey(provider, (BYTE*)&key_blob, - sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, - 0, CRYPT_IPSEC_HMAC_KEY, &hkey); - /* CRYPT_IPSEC_HMAC_KEY is not supported on W2K and lower */ - todo_wine - ok(ret || - broken(!ret && GetLastError() == NTE_BAD_FLAGS), - "CryptImportKey error %08x\n", GetLastError()); - - pCryptDestroyKey(hkey); - pCryptReleaseContext(provider, 0); + if (ret) + { + key_blob.header.bType = PLAINTEXTKEYBLOB; + key_blob.header.bVersion = CUR_BLOB_VERSION; + key_blob.header.reserved = 0; + key_blob.header.aiKeyAlg = CALG_RC2; + key_blob.key_size = sizeof(key); + memcpy(key_blob.key_data, key, key_length); + + /* Importing a 16-byte key works with the default provider. */ + SetLastError(0xdeadbeef); + ret = pCryptImportKey(provider, (BYTE*)&key_blob, + sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, + 0, CRYPT_IPSEC_HMAC_KEY, &hkey); + /* CRYPT_IPSEC_HMAC_KEY is not supported on W2K and lower */ + todo_wine + ok(ret || + broken(!ret && GetLastError() == NTE_BAD_FLAGS), + "CryptImportKey error %08x\n", GetLastError()); + + if (ret) + pCryptDestroyKey(hkey); + pCryptReleaseContext(provider, 0); + } SetLastError(0xdeadbeef); ret = pCryptAcquireContextA(&provider, NULL, MS_DEF_PROV, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT); ok(ret, "CryptAcquireContext error %08x\n", GetLastError()); - /* Importing a 16-byte key doesn't work with the base provider.. */ - SetLastError(0xdeadbeef); - ret = pCryptImportKey(provider, (BYTE*)&key_blob, - sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, - 0, 0, &hkey); - ok(!ret && (GetLastError() == NTE_BAD_DATA || - GetLastError() == NTE_BAD_LEN || /* Win7 */ - GetLastError() == NTE_BAD_TYPE || /* W2K */ - GetLastError() == NTE_PERM), /* Win9x, WinMe and NT4 */ - "unexpected error %08x\n", GetLastError()); - /* but importing an 56-bit (7-byte) key does.. */ - key_blob.key_size = 7; - SetLastError(0xdeadbeef); - ret = pCryptImportKey(provider, (BYTE*)&key_blob, - sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, - 0, 0, &hkey); - ok(ret || - broken(!ret && GetLastError() == NTE_BAD_TYPE) || /* W2K */ - broken(!ret && GetLastError() == NTE_PERM), /* Win9x, WinMe and NT4 */ - "CryptAcquireContext error %08x\n", GetLastError()); - pCryptDestroyKey(hkey); - /* as does importing a 16-byte key with the base provider when - * CRYPT_IPSEC_HMAC_KEY is specified. - */ - key_blob.key_size = sizeof(key); - SetLastError(0xdeadbeef); - ret = pCryptImportKey(provider, (BYTE*)&key_blob, - sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, - 0, CRYPT_IPSEC_HMAC_KEY, &hkey); - /* CRYPT_IPSEC_HMAC_KEY is not supported on W2K and lower */ - todo_wine - ok(ret || - broken(!ret && GetLastError() == NTE_BAD_FLAGS), - "CryptImportKey error %08x\n", GetLastError()); - pCryptDestroyKey(hkey); - - pCryptReleaseContext(provider, 0); + if (ret) + { + /* Importing a 16-byte key doesn't work with the base provider.. */ + SetLastError(0xdeadbeef); + ret = pCryptImportKey(provider, (BYTE*)&key_blob, + sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, + 0, 0, &hkey); + ok(!ret && (GetLastError() == NTE_BAD_DATA || + GetLastError() == NTE_BAD_LEN || /* Win7 */ + GetLastError() == NTE_BAD_TYPE || /* W2K */ + GetLastError() == NTE_PERM), /* Win9x, WinMe and NT4 */ + "unexpected error %08x\n", GetLastError()); + /* but importing an 56-bit (7-byte) key does.. */ + key_blob.key_size = 7; + SetLastError(0xdeadbeef); + ret = pCryptImportKey(provider, (BYTE*)&key_blob, + sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, + 0, 0, &hkey); + ok(ret || + broken(!ret && GetLastError() == NTE_BAD_TYPE) || /* W2K */ + broken(!ret && GetLastError() == NTE_PERM), /* Win9x, WinMe and NT4 */ + "CryptAcquireContext error %08x\n", GetLastError()); + if (ret) + pCryptDestroyKey(hkey); + /* as does importing a 16-byte key with the base provider when + * CRYPT_IPSEC_HMAC_KEY is specified. + */ + key_blob.key_size = sizeof(key); + SetLastError(0xdeadbeef); + ret = pCryptImportKey(provider, (BYTE*)&key_blob, + sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, + 0, CRYPT_IPSEC_HMAC_KEY, &hkey); + /* CRYPT_IPSEC_HMAC_KEY is not supported on W2K and lower */ + todo_wine + ok(ret || + broken(!ret && GetLastError() == NTE_BAD_FLAGS), + "CryptImportKey error %08x\n", GetLastError()); + if (ret) + pCryptDestroyKey(hkey); + + pCryptReleaseContext(provider, 0); + } key_blob.key_size = sizeof(key); SetLastError(0xdeadbeef); @@ -1027,34 +1035,39 @@ static void test_rc2_keylen(void) PROV_RSA_FULL, CRYPT_VERIFYCONTEXT); ok(ret, "CryptAcquireContext error %08x\n", GetLastError()); - /* Importing a 16-byte key also works with the default provider when - * CRYPT_IPSEC_HMAC_KEY is specified. - */ - SetLastError(0xdeadbeef); - ret = pCryptImportKey(provider, (BYTE*)&key_blob, - sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, - 0, CRYPT_IPSEC_HMAC_KEY, &hkey); - todo_wine - ok(ret || - broken(!ret && GetLastError() == NTE_BAD_FLAGS), - "CryptImportKey error %08x\n", GetLastError()); - pCryptDestroyKey(hkey); - - /* There is no apparent limit to the size of the input key when - * CRYPT_IPSEC_HMAC_KEY is specified. - */ - key_blob.key_size = sizeof(key_blob.key_data); - SetLastError(0xdeadbeef); - ret = pCryptImportKey(provider, (BYTE*)&key_blob, - sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, - 0, CRYPT_IPSEC_HMAC_KEY, &hkey); - todo_wine - ok(ret || - broken(!ret && GetLastError() == NTE_BAD_FLAGS), - "CryptImportKey error %08x\n", GetLastError()); - pCryptDestroyKey(hkey); - - pCryptReleaseContext(provider, 0); + if (ret) + { + /* Importing a 16-byte key also works with the default provider when + * CRYPT_IPSEC_HMAC_KEY is specified. + */ + SetLastError(0xdeadbeef); + ret = pCryptImportKey(provider, (BYTE*)&key_blob, + sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, + 0, CRYPT_IPSEC_HMAC_KEY, &hkey); + todo_wine + ok(ret || + broken(!ret && GetLastError() == NTE_BAD_FLAGS), + "CryptImportKey error %08x\n", GetLastError()); + if (ret) + pCryptDestroyKey(hkey); + + /* There is no apparent limit to the size of the input key when + * CRYPT_IPSEC_HMAC_KEY is specified. + */ + key_blob.key_size = sizeof(key_blob.key_data); + SetLastError(0xdeadbeef); + ret = pCryptImportKey(provider, (BYTE*)&key_blob, + sizeof(BLOBHEADER)+sizeof(DWORD)+key_blob.key_size, + 0, CRYPT_IPSEC_HMAC_KEY, &hkey); + todo_wine + ok(ret || + broken(!ret && GetLastError() == NTE_BAD_FLAGS), + "CryptImportKey error %08x\n", GetLastError()); + if (ret) + pCryptDestroyKey(hkey); + + pCryptReleaseContext(provider, 0); + } } START_TEST(crypt)

1 0

Juan Lang : crypt32: When removing contexts from a list, make sure the context no longer references the list.
by Alexandre Julliard Nov. 4, 2009

Nov. 4, 2009

Module: wine Branch: master Commit: fb5e0d8a4d85483a3093cf0fdad21eb0da7264a4 URL: http://source.winehq.org/git/wine.git/?a=commit;h=fb5e0d8a4d85483a3093cf0fd… Author: Juan Lang <juan.lang(a)gmail.com> Date: Fri Oct 30 16:43:24 2009 -0700 crypt32: When removing contexts from a list, make sure the context no longer references the list. --- dlls/crypt32/context.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/dlls/crypt32/context.c b/dlls/crypt32/context.c index 9ada6bf..20fe024 100644 --- a/dlls/crypt32/context.c +++ b/dlls/crypt32/context.c @@ -289,6 +289,7 @@ void ContextList_Delete(struct ContextList *list, void *context) EnterCriticalSection(&list->cs); list_remove(entry); LeaveCriticalSection(&list->cs); + list_init(entry); list->contextInterface->free(context); }

1 0

Juan Lang : crypt32: Correct reference counting when deleting contexts from collections.
by Alexandre Julliard Nov. 4, 2009

Nov. 4, 2009

Module: wine Branch: master Commit: 976c6ff3f8fd679b70f3a92e3fa71c58256ec28f URL: http://source.winehq.org/git/wine.git/?a=commit;h=976c6ff3f8fd679b70f3a92e3… Author: Juan Lang <juan.lang(a)gmail.com> Date: Tue Nov 3 17:04:32 2009 -0800 crypt32: Correct reference counting when deleting contexts from collections. --- dlls/crypt32/collectionstore.c | 34 ++++++++++++++++++++++++++++------ dlls/crypt32/tests/store.c | 22 ++++++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/dlls/crypt32/collectionstore.c b/dlls/crypt32/collectionstore.c index 12eb7c3..e55c1bf 100644 --- a/dlls/crypt32/collectionstore.c +++ b/dlls/crypt32/collectionstore.c @@ -255,11 +255,19 @@ static BOOL CRYPT_CollectionDeleteCert(PWINECRYPT_CERTSTORE store, void *pCertContext) { BOOL ret; + PCCERT_CONTEXT linked; TRACE("(%p, %p)\n", store, pCertContext); - ret = CertDeleteCertificateFromStore( - Context_GetLinkedContext(pCertContext, sizeof(CERT_CONTEXT))); + /* Deleting the linked context results in its ref count getting + * decreased, but the caller of this (CertDeleteCertificateFromStore) also + * decreases pCertContext's ref count, by calling + * CertFreeCertificateContext. Increase ref count of linked context to + * compensate. + */ + linked = Context_GetLinkedContext(pCertContext, sizeof(CERT_CONTEXT)); + CertDuplicateCertificateContext(linked); + ret = CertDeleteCertificateFromStore(linked); return ret; } @@ -333,11 +341,18 @@ static BOOL CRYPT_CollectionDeleteCRL(PWINECRYPT_CERTSTORE store, void *pCrlContext) { BOOL ret; + PCCRL_CONTEXT linked; TRACE("(%p, %p)\n", store, pCrlContext); - ret = CertDeleteCRLFromStore( - Context_GetLinkedContext(pCrlContext, sizeof(CRL_CONTEXT))); + /* Deleting the linked context results in its ref count getting + * decreased, but the caller of this (CertDeleteCRLFromStore) also + * decreases pCrlContext's ref count, by calling CertFreeCRLContext. + * Increase ref count of linked context to compensate. + */ + linked = Context_GetLinkedContext(pCrlContext, sizeof(CRL_CONTEXT)); + CertDuplicateCRLContext(linked); + ret = CertDeleteCRLFromStore(linked); return ret; } @@ -411,11 +426,18 @@ static BOOL CRYPT_CollectionDeleteCTL(PWINECRYPT_CERTSTORE store, void *pCtlContext) { BOOL ret; + PCCTL_CONTEXT linked; TRACE("(%p, %p)\n", store, pCtlContext); - ret = CertDeleteCTLFromStore( - Context_GetLinkedContext(pCtlContext, sizeof(CTL_CONTEXT))); + /* Deleting the linked context results in its ref count getting + * decreased, but the caller of this (CertDeleteCTLFromStore) also + * decreases pCtlContext's ref count, by calling CertFreeCTLContext. + * Increase ref count of linked context to compensate. + */ + linked = Context_GetLinkedContext(pCtlContext, sizeof(CTL_CONTEXT)); + CertDuplicateCTLContext(linked); + ret = CertDeleteCTLFromStore(linked); return ret; } diff --git a/dlls/crypt32/tests/store.c b/dlls/crypt32/tests/store.c index 88f53ed..1f6609c 100644 --- a/dlls/crypt32/tests/store.c +++ b/dlls/crypt32/tests/store.c @@ -574,6 +574,28 @@ static void testCollectionStore(void) CertCloseStore(collection, 0); CertCloseStore(store2, 0); CertCloseStore(store1, 0); + + /* Test adding certificates to and deleting certificates from collections. + */ + store1 = CertOpenSystemStoreA(0, "My"); + collection = CertOpenStore(CERT_STORE_PROV_COLLECTION, 0, 0, + CERT_STORE_CREATE_NEW_FLAG, NULL); + + ret = CertAddEncodedCertificateToStore(store1, X509_ASN_ENCODING, + bigCert, sizeof(bigCert), CERT_STORE_ADD_ALWAYS, &context); + ok(ret, "CertAddEncodedCertificateToStore failed: %08x\n", GetLastError()); + CertDeleteCertificateFromStore(context); + + CertAddStoreToCollection(collection, store1, + CERT_PHYSICAL_STORE_ADD_ENABLE_FLAG, 0); + + ret = CertAddEncodedCertificateToStore(collection, X509_ASN_ENCODING, + bigCert, sizeof(bigCert), CERT_STORE_ADD_ALWAYS, &context); + ok(ret, "CertAddEncodedCertificateToStore failed: %08x\n", GetLastError()); + CertDeleteCertificateFromStore(context); + + CertCloseStore(collection, 0); + CertCloseStore(store1, 0); } /* Looks for the property with ID propID in the buffer buf. Returns a pointer

1 0

Juan Lang : crypt32: Add return value to Context_Release to allow detecting reference counting errors .
by Alexandre Julliard Nov. 4, 2009

Nov. 4, 2009

Module: wine Branch: master Commit: 40855cae977eeab3e2781616c85ceba2ef82a6ba URL: http://source.winehq.org/git/wine.git/?a=commit;h=40855cae977eeab3e2781616c… Author: Juan Lang <juan.lang(a)gmail.com> Date: Fri Oct 30 15:06:39 2009 -0700 crypt32: Add return value to Context_Release to allow detecting reference counting errors. --- dlls/crypt32/cert.c | 6 ++++-- dlls/crypt32/context.c | 8 ++++++-- dlls/crypt32/crl.c | 6 ++++-- dlls/crypt32/crypt32_private.h | 3 ++- dlls/crypt32/ctl.c | 6 ++++-- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/dlls/crypt32/cert.c b/dlls/crypt32/cert.c index 2a65290..4f3df32 100644 --- a/dlls/crypt32/cert.c +++ b/dlls/crypt32/cert.c @@ -180,12 +180,14 @@ static void CertDataContext_Free(void *context) BOOL WINAPI CertFreeCertificateContext(PCCERT_CONTEXT pCertContext) { + BOOL ret = TRUE; + TRACE("(%p)\n", pCertContext); if (pCertContext) - Context_Release((void *)pCertContext, sizeof(CERT_CONTEXT), + ret = Context_Release((void *)pCertContext, sizeof(CERT_CONTEXT), CertDataContext_Free); - return TRUE; + return ret; } DWORD WINAPI CertEnumCertificateContextProperties(PCCERT_CONTEXT pCertContext, diff --git a/dlls/crypt32/context.c b/dlls/crypt32/context.c index bb87799..9ada6bf 100644 --- a/dlls/crypt32/context.c +++ b/dlls/crypt32/context.c @@ -135,11 +135,14 @@ PCONTEXT_PROPERTY_LIST Context_GetProperties(const void *context, size_t context ((PDATA_CONTEXT)ptr)->properties : NULL; } -void Context_Release(void *context, size_t contextSize, +BOOL Context_Release(void *context, size_t contextSize, ContextFreeFunc dataContextFree) { PBASE_CONTEXT base = BASE_CONTEXT_FROM_CONTEXT(context, contextSize); + BOOL ret = TRUE; + if (base->ref <= 0) + return FALSE; if (InterlockedDecrement(&base->ref) == 0) { TRACE("freeing %p\n", context); @@ -153,7 +156,7 @@ void Context_Release(void *context, size_t contextSize, /* The linked context is of the same type as this, so release * it as well, using the same offset and data free function. */ - Context_Release(CONTEXT_FROM_BASE_CONTEXT( + ret = Context_Release(CONTEXT_FROM_BASE_CONTEXT( ((PLINK_CONTEXT)base)->linked, contextSize), contextSize, dataContextFree); break; @@ -164,6 +167,7 @@ void Context_Release(void *context, size_t contextSize, } else TRACE("%p's ref count is %d\n", context, base->ref); + return ret; } void Context_CopyProperties(const void *to, const void *from, diff --git a/dlls/crypt32/crl.c b/dlls/crypt32/crl.c index 80fe86f..522aadf 100644 --- a/dlls/crypt32/crl.c +++ b/dlls/crypt32/crl.c @@ -243,12 +243,14 @@ static void CrlDataContext_Free(void *context) BOOL WINAPI CertFreeCRLContext( PCCRL_CONTEXT pCrlContext) { + BOOL ret = TRUE; + TRACE("(%p)\n", pCrlContext); if (pCrlContext) - Context_Release((void *)pCrlContext, sizeof(CRL_CONTEXT), + ret = Context_Release((void *)pCrlContext, sizeof(CRL_CONTEXT), CrlDataContext_Free); - return TRUE; + return ret; } DWORD WINAPI CertEnumCRLContextProperties(PCCRL_CONTEXT pCRLContext, diff --git a/dlls/crypt32/crypt32_private.h b/dlls/crypt32/crypt32_private.h index 8c5b13b..dd96e0f 100644 --- a/dlls/crypt32/crypt32_private.h +++ b/dlls/crypt32/crypt32_private.h @@ -346,8 +346,9 @@ typedef void (*ContextFreeFunc)(void *context); /* Decrements context's ref count. If context is a link context, releases its * linked context as well. * If a data context has its ref count reach 0, calls dataContextFree on it. + * Returns FALSE if the reference count is <= 0 when called. */ -void Context_Release(void *context, size_t contextSize, +BOOL Context_Release(void *context, size_t contextSize, ContextFreeFunc dataContextFree); /** diff --git a/dlls/crypt32/ctl.c b/dlls/crypt32/ctl.c index 1bd60e7..e2511fa 100644 --- a/dlls/crypt32/ctl.c +++ b/dlls/crypt32/ctl.c @@ -472,12 +472,14 @@ static void CTLDataContext_Free(void *context) BOOL WINAPI CertFreeCTLContext(PCCTL_CONTEXT pCTLContext) { + BOOL ret = TRUE; + TRACE("(%p)\n", pCTLContext); if (pCTLContext) - Context_Release((void *)pCTLContext, sizeof(CTL_CONTEXT), + ret = Context_Release((void *)pCTLContext, sizeof(CTL_CONTEXT), CTLDataContext_Free); - return TRUE; + return ret; } DWORD WINAPI CertEnumCTLContextProperties(PCCTL_CONTEXT pCTLContext,

1 0

Juan Lang : crypt32: Propagate errors from CertFree*Context to CertDelete*FromStore.
by Alexandre Julliard Nov. 4, 2009

Nov. 4, 2009

Module: wine Branch: master Commit: 92324ab38e1fc8423ae4a78456bca62e5c7b7c56 URL: http://source.winehq.org/git/wine.git/?a=commit;h=92324ab38e1fc8423ae4a7845… Author: Juan Lang <juan.lang(a)gmail.com> Date: Fri Oct 30 15:18:53 2009 -0700 crypt32: Propagate errors from CertFree*Context to CertDelete*FromStore. --- dlls/crypt32/ctl.c | 8 +++----- dlls/crypt32/store.c | 15 +++++---------- dlls/crypt32/tests/store.c | 1 + 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/dlls/crypt32/ctl.c b/dlls/crypt32/ctl.c index e2511fa..44b517a 100644 --- a/dlls/crypt32/ctl.c +++ b/dlls/crypt32/ctl.c @@ -309,10 +309,7 @@ BOOL WINAPI CertDeleteCTLFromStore(PCCTL_CONTEXT pCtlContext) if (!pCtlContext) ret = TRUE; else if (!pCtlContext->hCertStore) - { - ret = TRUE; - CertFreeCTLContext(pCtlContext); - } + ret = CertFreeCTLContext(pCtlContext); else { PWINECRYPT_CERTSTORE hcs = pCtlContext->hCertStore; @@ -321,7 +318,8 @@ BOOL WINAPI CertDeleteCTLFromStore(PCCTL_CONTEXT pCtlContext) ret = FALSE; else ret = hcs->ctls.deleteContext(hcs, (void *)pCtlContext); - CertFreeCTLContext(pCtlContext); + if (ret) + ret = CertFreeCTLContext(pCtlContext); } return ret; } diff --git a/dlls/crypt32/store.c b/dlls/crypt32/store.c index 8acc6f6..a529ac6 100644 --- a/dlls/crypt32/store.c +++ b/dlls/crypt32/store.c @@ -966,10 +966,7 @@ BOOL WINAPI CertDeleteCertificateFromStore(PCCERT_CONTEXT pCertContext) if (!pCertContext) ret = TRUE; else if (!pCertContext->hCertStore) - { - ret = TRUE; - CertFreeCertificateContext(pCertContext); - } + ret = CertFreeCertificateContext(pCertContext); else { PWINECRYPT_CERTSTORE hcs = pCertContext->hCertStore; @@ -979,7 +976,7 @@ BOOL WINAPI CertDeleteCertificateFromStore(PCCERT_CONTEXT pCertContext) else ret = hcs->certs.deleteContext(hcs, (void *)pCertContext); if (ret) - CertFreeCertificateContext(pCertContext); + ret = CertFreeCertificateContext(pCertContext); } return ret; } @@ -1105,10 +1102,7 @@ BOOL WINAPI CertDeleteCRLFromStore(PCCRL_CONTEXT pCrlContext) if (!pCrlContext) ret = TRUE; else if (!pCrlContext->hCertStore) - { - ret = TRUE; - CertFreeCRLContext(pCrlContext); - } + ret = CertFreeCRLContext(pCrlContext); else { PWINECRYPT_CERTSTORE hcs = pCrlContext->hCertStore; @@ -1117,7 +1111,8 @@ BOOL WINAPI CertDeleteCRLFromStore(PCCRL_CONTEXT pCrlContext) ret = FALSE; else ret = hcs->crls.deleteContext(hcs, (void *)pCrlContext); - CertFreeCRLContext(pCrlContext); + if (ret) + ret = CertFreeCRLContext(pCrlContext); } return ret; } diff --git a/dlls/crypt32/tests/store.c b/dlls/crypt32/tests/store.c index ae10fbc..88f53ed 100644 --- a/dlls/crypt32/tests/store.c +++ b/dlls/crypt32/tests/store.c @@ -237,6 +237,7 @@ static void testMemStore(void) GetLastError()); /* try deleting a copy */ ret = CertDeleteCertificateFromStore(copy); + todo_wine ok(ret, "CertDeleteCertificateFromStore failed: %08x\n", GetLastError()); /* check that the store is empty */

1 0