user: Use DDE critical section exclusively for instance list protection

Dmitry Timoshkov dmitry at codeweavers.com
Wed Nov 15 07:22:35 CST 2006


Hello,

DDE instance and conversation structure accesses (in WDML_GetInstance()
and WDML_GetConv()) are restricted to the thread that created them, so
it's not practical to use the instance list lock (as clearly stated in
the dde_private.h comment) to protect accesses to them.

Changelog:
    user: Use DDE critical section exclusively for instance list protection.

---
 dlls/user/dde_client.c  |   76 ++++++-------------------------------------
 dlls/user/dde_misc.c    |   83 ++++++++++++++++-------------------------------
 dlls/user/dde_private.h |    2 -
 dlls/user/dde_server.c  |   43 +++++++-----------------
 4 files changed, 52 insertions(+), 152 deletions(-)

diff --git a/dlls/user/dde_client.c b/dlls/user/dde_client.c
index bcf8b33..243ea45 100644
--- a/dlls/user/dde_client.c
+++ b/dlls/user/dde_client.c
@@ -103,20 +103,16 @@ HCONV WINAPI DdeConnect(DWORD idInst, HS
 
     TRACE("(0x%x,%p,%p,%p)\n", idInst, hszService, hszTopic, pCC);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     pInstance = WDML_GetInstance(idInst);
     if (!pInstance)
-    {
-	goto theEnd;
-    }
+        return NULL;
 
     /* make sure this conv is never created */
     pConv = WDML_FindConv(pInstance, WDML_CLIENT_SIDE, hszService, hszTopic);
     if (pConv != NULL)
     {
 	ERR("This Conv already exists: (%p)\n", pConv);
-	goto theEnd;
+	return NULL;
     }
 
     /* we need to establish a conversation with
@@ -178,14 +174,10 @@ HCONV WINAPI DdeConnect(DWORD idInst, HS
 	if (!aTpc) goto theEnd;
     }
 
-    LeaveCriticalSection(&WDML_CritSect);
-
     /* note: sent messages shall not use packing */
     SendMessageTimeoutW( HWND_BROADCAST, WM_DDE_INITIATE, (WPARAM)hwndClient, MAKELPARAM(aSrv, aTpc),
                          SMTO_ABORTIFHUNG, 2000, NULL );
 
-    EnterCriticalSection(&WDML_CritSect);
-
     pInstance = WDML_GetInstance(idInst);
     if (!pInstance)
     {
@@ -218,7 +210,6 @@ HCONV WINAPI DdeConnect(DWORD idInst, HS
     }
 
  theEnd:
-    LeaveCriticalSection(&WDML_CritSect);
 
     if (aSrv) GlobalDeleteAtom(aSrv);
     if (aTpc) GlobalDeleteAtom(aTpc);
@@ -237,7 +228,6 @@ HCONV WINAPI DdeReconnect(HCONV hConv)
 
     TRACE("(%p)\n", hConv);
 
-    EnterCriticalSection(&WDML_CritSect);
     pConv = WDML_GetConv(hConv, FALSE);
     if (pConv != NULL && (pConv->wStatus & ST_CLIENT))
     {
@@ -261,14 +251,10 @@ HCONV WINAPI DdeReconnect(HCONV hConv)
 	    aTpc = WDML_MakeAtomFromHsz(pConv->hszTopic);
 	    if (!aSrv || !aTpc)	goto theEnd;
 
-	    LeaveCriticalSection(&WDML_CritSect);
-
             /* note: sent messages shall not use packing */
 	    ret = SendMessageW(hwndServer, WM_DDE_INITIATE, (WPARAM)hwndClient,
                                MAKELPARAM(aSrv, aTpc));
 
-	    EnterCriticalSection(&WDML_CritSect);
-
 	    pConv = WDML_GetConv(hConv, FALSE);
 	    if (pConv == NULL)
 	    {
@@ -300,7 +286,6 @@ HCONV WINAPI DdeReconnect(HCONV hConv)
     }
 
  theEnd:
-    LeaveCriticalSection(&WDML_CritSect);
 
     if (aSrv) GlobalDeleteAtom(aSrv);
     if (aTpc) GlobalDeleteAtom(aTpc);
@@ -1037,12 +1022,9 @@ static HDDEDATA WDML_SyncWaitTransaction
 	    {
                 HDDEDATA hdd;
 
-                EnterCriticalSection(&WDML_CritSect);
-
                 pConv = WDML_GetConv(hConv, FALSE);
                 if (pConv == NULL)
                 {
-                    LeaveCriticalSection(&WDML_CritSect);
                     /* conversation no longer available... return failure */
                     return 0;
                 }
@@ -1061,13 +1043,11 @@ static HDDEDATA WDML_SyncWaitTransaction
                     if (ret)
                     {
                         pConv->instance->lastError = hdd ? DMLERR_NO_ERROR : DMLERR_NOTPROCESSED;
-                        LeaveCriticalSection(&WDML_CritSect);
                         return hdd;
                     }
                 }
                 else
                 {
-                    LeaveCriticalSection(&WDML_CritSect);
                     DispatchMessageW(&msg);
                 }
             }
@@ -1076,8 +1056,6 @@ static HDDEDATA WDML_SyncWaitTransaction
 
     TRACE("Timeout !!\n");
 
-    EnterCriticalSection(&WDML_CritSect);
-
     pConv = WDML_GetConv(hConv, FALSE);
     if (pConv != NULL)
     {
@@ -1096,7 +1074,6 @@ static HDDEDATA WDML_SyncWaitTransaction
 	    pConv->instance->lastError = err;
 	}
     }
-    LeaveCriticalSection(&WDML_CritSect);
 
     return 0;
 }
@@ -1120,13 +1097,11 @@ HDDEDATA WINAPI DdeClientTransaction(LPB
 	return 0;
     }
 
-    EnterCriticalSection(&WDML_CritSect);
-
     pConv = WDML_GetConv(hConv, TRUE);
     if (pConv == NULL)
     {
 	/* cannot set error... cannot get back to DDE instance */
-	goto theError;
+        return 0;
     }
 
     switch (wType)
@@ -1145,7 +1120,7 @@ HDDEDATA WINAPI DdeClientTransaction(LPB
 	if (pData)
 	{
 	    pConv->instance->lastError = DMLERR_INVALIDPARAMETER;
-	    goto theError;
+            return 0;
 	}
 	pXAct = WDML_ClientQueueAdvise(pConv, wType, wFmt, hszItem);
 	break;
@@ -1153,7 +1128,7 @@ HDDEDATA WINAPI DdeClientTransaction(LPB
 	if (pData)
 	{
 	    pConv->instance->lastError = DMLERR_INVALIDPARAMETER;
-	    goto theError;
+            return 0;
 	}
 	pXAct = WDML_ClientQueueUnadvise(pConv, wFmt, hszItem);
 	break;
@@ -1161,7 +1136,7 @@ HDDEDATA WINAPI DdeClientTransaction(LPB
 	if (pData)
 	{
 	    pConv->instance->lastError = DMLERR_INVALIDPARAMETER;
-	    goto theError;
+            return 0;
 	}
 	pXAct = WDML_ClientQueueRequest(pConv, wFmt, hszItem);
 	break;
@@ -1169,13 +1144,13 @@ HDDEDATA WINAPI DdeClientTransaction(LPB
         FIXME("Unknown transaction type %04x\n", wType);
 	/* unknown transaction type */
 	pConv->instance->lastError = DMLERR_INVALIDPARAMETER;
-	goto theError;
+        return 0;
     }
 
     if (pXAct == NULL)
     {
 	pConv->instance->lastError = DMLERR_MEMORY_ERROR;
-	goto theError;
+        return 0;
     }
 
     WDML_QueueTransaction(pConv, pXAct);
@@ -1188,7 +1163,7 @@ HDDEDATA WINAPI DdeClientTransaction(LPB
 	WDML_UnQueueTransaction(pConv, pXAct);
 	WDML_FreeTransaction(pConv->instance, pXAct, TRUE);
         pConv->instance->lastError = DMLERR_POSTMSG_FAILED;
-	goto theError;
+        return 0;
     }
     pXAct->dwTimeout = dwTimeout;
     /* FIXME: should set the app bits on *pdwResult */
@@ -1202,22 +1177,9 @@ HDDEDATA WINAPI DdeClientTransaction(LPB
 	hDdeData = (HDDEDATA)1;
     }
     else
-    {
-	DWORD	count, i;
-
-	count = WDML_CritSect.RecursionCount;
-	for (i = 0; i < count; i++)
-	    LeaveCriticalSection(&WDML_CritSect);
-	hDdeData = WDML_SyncWaitTransactionReply((HCONV)pConv, dwTimeout, pXAct, pdwResult);
-	for (i = 0; i < count; i++)
-	    EnterCriticalSection(&WDML_CritSect);
-    }
-    LeaveCriticalSection(&WDML_CritSect);
+        hDdeData = WDML_SyncWaitTransactionReply(hConv, dwTimeout, pXAct, pdwResult);
 
     return hDdeData;
- theError:
-    LeaveCriticalSection(&WDML_CritSect);
-    return 0;
 }
 
 /*****************************************************************
@@ -1231,7 +1193,6 @@ BOOL WINAPI DdeAbandonTransaction(DWORD 
 
     TRACE("(%08x,%p,%08x);\n", idInst, hConv, idTransaction);
 
-    EnterCriticalSection(&WDML_CritSect);
     if ((pInstance = WDML_GetInstance(idInst)))
     {
         if (hConv)
@@ -1265,7 +1226,6 @@ BOOL WINAPI DdeAbandonTransaction(DWORD 
             }
         }
     }
-    LeaveCriticalSection(&WDML_CritSect);
 
     return TRUE;
 }
@@ -1336,8 +1296,6 @@ static LRESULT CALLBACK WDML_ClientProc(
 
     if (iMsg >= WM_DDE_FIRST && iMsg <= WM_DDE_LAST)
     {
-	EnterCriticalSection(&WDML_CritSect);
-
 	pConv = WDML_GetConvFromWnd(hwnd);
 
 	if (pConv)
@@ -1353,7 +1311,6 @@ static LRESULT CALLBACK WDML_ClientProc(
 	    WDML_HandleReply(pConv, &msg, &hdd, NULL);
 	}
 
-	LeaveCriticalSection(&WDML_CritSect);
 	return 0;
     }
 
@@ -1368,7 +1325,6 @@ BOOL WINAPI DdeDisconnect(HCONV hConv)
 {
     WDML_CONV*	pConv = NULL;
     WDML_XACT*	pXAct;
-    DWORD	count, i;
     BOOL	ret = FALSE;
 
     TRACE("(%p)\n", hConv);
@@ -1379,7 +1335,6 @@ BOOL WINAPI DdeDisconnect(HCONV hConv)
 	return FALSE;
     }
 
-    EnterCriticalSection(&WDML_CritSect);
     pConv = WDML_GetConv(hConv, TRUE);
     if (pConv != NULL)
     {
@@ -1389,19 +1344,13 @@ BOOL WINAPI DdeDisconnect(HCONV hConv)
             pXAct = WDML_ClientQueueTerminate(pConv);
             if (pXAct != NULL)
             {
-                count = WDML_CritSect.RecursionCount;
-                for (i = 0; i < count; i++)
-                    LeaveCriticalSection(&WDML_CritSect);
                 if (PostMessageW(pConv->hwndServer, pXAct->ddeMsg,
                                  (WPARAM)pConv->hwndClient, pXAct->lParam))
                 {
                     WDML_SyncWaitTransactionReply(hConv, 10000, pXAct, NULL);
                     ret = TRUE;
                 }
-                for (i = 0; i < count; i++)
-                    EnterCriticalSection(&WDML_CritSect);
-
-                if (!ret)
+                else
                     pConv->instance->lastError = DMLERR_POSTMSG_FAILED;
 
                 WDML_FreeTransaction(pConv->instance, pXAct, TRUE);
@@ -1414,7 +1363,6 @@ BOOL WINAPI DdeDisconnect(HCONV hConv)
             }
         }
     }
-    LeaveCriticalSection(&WDML_CritSect);
 
     return ret;
 }
@@ -1429,12 +1377,10 @@ BOOL WINAPI DdeImpersonateClient(HCONV h
 
     TRACE("(%p)\n", hConv);
 
-    EnterCriticalSection(&WDML_CritSect);
     pConv = WDML_GetConv(hConv, TRUE);
     if (pConv)
     {
 	ret = ImpersonateDdeClientWindow(pConv->hwndClient, pConv->hwndServer);
     }
-    LeaveCriticalSection(&WDML_CritSect);
     return ret;
 }
diff --git a/dlls/user/dde_misc.c b/dlls/user/dde_misc.c
index e707c09..0afb2ed 100644
--- a/dlls/user/dde_misc.c
+++ b/dlls/user/dde_misc.c
@@ -51,13 +51,15 @@ static WDML_INSTANCE*	WDML_InstanceList 
 static LONG		WDML_MaxInstanceID = 0;  /* OK for present, have to worry about wrap-around later */
 const WCHAR		WDML_szEventClass[] = {'W','i','n','e','D','d','e','E','v','e','n','t','C','l','a','s','s',0};
 
+/* protection for instance list */
+static CRITICAL_SECTION WDML_CritSect;
 static CRITICAL_SECTION_DEBUG critsect_debug =
 {
     0, 0, &WDML_CritSect,
     { &critsect_debug.ProcessLocksList, &critsect_debug.ProcessLocksList },
       0, 0, { (DWORD_PTR)(__FILE__ ": WDML_CritSect") }
 };
-CRITICAL_SECTION WDML_CritSect = { &critsect_debug, -1, 0, 0, 0, 0 };
+static CRITICAL_SECTION WDML_CritSect = { &critsect_debug, -1, 0, 0, 0, 0 };
 
 /* ================================================================
  *
@@ -632,14 +634,11 @@ BOOL WINAPI DdeUninitialize(DWORD idInst
 
     TRACE("(%d)\n", idInst);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     /*  First check instance
      */
     pInstance = WDML_GetInstance(idInst);
     if (pInstance == NULL)
     {
-	LeaveCriticalSection(&WDML_CritSect);
 	/*
 	 *	Needs something here to record NOT_INITIALIZED ready for DdeGetLastError
 	 */
@@ -682,10 +681,10 @@ BOOL WINAPI DdeUninitialize(DWORD idInst
 	for (inst = WDML_InstanceList; inst->next != pInstance; inst = inst->next);
 	inst->next = pInstance->next;
     }
-    /* leave crit sect and release the heap entry
+    /* release the heap entry
      */
     HeapFree(GetProcessHeap(), 0, pInstance);
-    LeaveCriticalSection(&WDML_CritSect);
+
     return TRUE;
 }
 
@@ -706,7 +705,9 @@ void WDML_NotifyThreadDetach(void)
 	next = pInstance->next;
 	if (pInstance->threadID == tid)
 	{
+            LeaveCriticalSection(&WDML_CritSect);
 	    DdeUninitialize(pInstance->instanceID);
+            EnterCriticalSection(&WDML_CritSect);
 	}
     }
     LeaveCriticalSection(&WDML_CritSect);
@@ -725,6 +726,7 @@ HDDEDATA 	WDML_InvokeCallback(WDML_INSTA
 
     if (pInstance == NULL)
 	return NULL;
+
     TRACE("invoking CB%d[%p] (%x %x %p %p %p %p %lx %lx)\n",
 	  pInstance->win16 ? 16 : 32, pInstance->callback, uType, uFmt,
 	  hConv, hsz1, hsz2, hdata, dwData1, dwData2);
@@ -752,6 +754,8 @@ WDML_INSTANCE*	WDML_GetInstance(DWORD in
 {
     WDML_INSTANCE*	pInstance;
 
+    EnterCriticalSection(&WDML_CritSect);
+
     for (pInstance = WDML_InstanceList; pInstance != NULL; pInstance = pInstance->next)
     {
 	if (pInstance->instanceID == instId)
@@ -761,11 +765,15 @@ WDML_INSTANCE*	WDML_GetInstance(DWORD in
 		FIXME("Tried to get instance from wrong thread\n");
 		continue;
 	    }
-	    return pInstance;
+	    break;
 	}
     }
-    TRACE("Instance entry missing\n");
-    return NULL;
+
+    LeaveCriticalSection(&WDML_CritSect);
+
+    if (!pInstance)
+        WARN("Instance entry missing for id %04x\n", instId);
+    return pInstance;
 }
 
 /******************************************************************
@@ -792,8 +800,6 @@ UINT WINAPI DdeGetLastError(DWORD idInst
     DWORD		error_code;
     WDML_INSTANCE*	pInstance;
 
-    EnterCriticalSection(&WDML_CritSect);
-
     /*  First check instance
      */
     pInstance = WDML_GetInstance(idInst);
@@ -807,7 +813,6 @@ UINT WINAPI DdeGetLastError(DWORD idInst
 	pInstance->lastError = 0;
     }
 
-    LeaveCriticalSection(&WDML_CritSect);
     return error_code;
 }
 
@@ -1018,8 +1023,6 @@ DWORD WINAPI DdeQueryStringA(DWORD idIns
 
     TRACE("(%d, %p, %p, %d, %d)\n", idInst, hsz, psz, cchMax, iCodePage);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     /*  First check instance
      */
     pInstance = WDML_GetInstance(idInst);
@@ -1028,7 +1031,6 @@ DWORD WINAPI DdeQueryStringA(DWORD idIns
 	if (iCodePage == 0) iCodePage = CP_WINANSI;
 	ret = WDML_QueryString(pInstance, hsz, psz, cchMax, iCodePage);
     }
-    LeaveCriticalSection(&WDML_CritSect);
 
     TRACE("returning %d (%s)\n", ret, debugstr_a(psz));
     return ret;
@@ -1045,8 +1047,6 @@ DWORD WINAPI DdeQueryStringW(DWORD idIns
 
     TRACE("(%d, %p, %p, %d, %d)\n", idInst, hsz, psz, cchMax, iCodePage);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     /*  First check instance
      */
     pInstance = WDML_GetInstance(idInst);
@@ -1055,7 +1055,6 @@ DWORD WINAPI DdeQueryStringW(DWORD idIns
 	if (iCodePage == 0) iCodePage = CP_WINUNICODE;
 	ret = WDML_QueryString(pInstance, hsz, psz, cchMax, iCodePage);
     }
-    LeaveCriticalSection(&WDML_CritSect);
 
     TRACE("returning %d (%s)\n", ret, debugstr_w(psz));
     return ret;
@@ -1100,8 +1099,6 @@ HSZ WINAPI DdeCreateStringHandleA(DWORD 
 
     TRACE("(%d,%s,%d)\n", idInst, debugstr_a(psz), codepage);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     pInstance = WDML_GetInstance(idInst);
     if (pInstance)
     {
@@ -1109,7 +1106,6 @@ HSZ WINAPI DdeCreateStringHandleA(DWORD 
 	hsz = WDML_CreateString(pInstance, psz, codepage);
     }
 
-    LeaveCriticalSection(&WDML_CritSect);
     return hsz;
 }
 
@@ -1132,15 +1128,12 @@ HSZ WINAPI DdeCreateStringHandleW(DWORD 
 
     TRACE("(%d,%s,%d)\n", idInst, debugstr_w(psz), codepage);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     pInstance = WDML_GetInstance(idInst);
     if (pInstance)
     {
 	if (codepage == 0) codepage = CP_WINUNICODE;
 	hsz = WDML_CreateString(pInstance, psz, codepage);
     }
-    LeaveCriticalSection(&WDML_CritSect);
 
     return hsz;
 }
@@ -1158,16 +1151,12 @@ BOOL WINAPI DdeFreeStringHandle(DWORD id
 
     TRACE("(%d,%p):\n", idInst, hsz);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     /*  First check instance
      */
     pInstance = WDML_GetInstance(idInst);
     if (pInstance)
 	ret = WDML_DecHSZ(pInstance, hsz);
 
-    LeaveCriticalSection(&WDML_CritSect);
-
     return ret;
 }
 
@@ -1185,15 +1174,12 @@ BOOL WINAPI DdeKeepStringHandle(DWORD id
 
     TRACE("(%d,%p):\n", idInst, hsz);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     /*  First check instance
      */
     pInstance = WDML_GetInstance(idInst);
     if (pInstance)
 	ret = WDML_IncHSZ(pInstance, hsz);
 
-    LeaveCriticalSection(&WDML_CritSect);
     return ret;
 }
 
@@ -1899,14 +1885,11 @@ BOOL WINAPI DdeEnableCallback(DWORD idIn
 
     TRACE("(%d, %p, %04x)\n", idInst, hConv, wCmd);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     pConv = WDML_GetConv(hConv, TRUE);
 
     if (pConv && pConv->instance->instanceID == idInst)
         ret = WDML_EnableCallback(pConv, wCmd);
 
-    LeaveCriticalSection(&WDML_CritSect);
     return ret;
 }
 
@@ -1922,16 +1905,17 @@ WDML_CONV*	WDML_GetConv(HCONV hConv, BOO
     /* FIXME: should do better checking */
     if (pConv == NULL || pConv->magic != WDML_CONV_MAGIC) return NULL;
 
-    if (checkConnected && !(pConv->wStatus & ST_CONNECTED))
+    if (!pConv->instance || pConv->instance->threadID != GetCurrentThreadId())
     {
-        WARN("found conv but ain't connected\n");
-        pConv->instance->lastError = DMLERR_NO_CONV_ESTABLISHED;
+        WARN("wrong thread ID\n");
+        pConv->instance->lastError = DMLERR_INVALIDPARAMETER; /* FIXME: check */
 	return NULL;
     }
-    if (!pConv->instance || GetCurrentThreadId() != pConv->instance->threadID)
+
+    if (checkConnected && !(pConv->wStatus & ST_CONNECTED))
     {
-        WARN("wrong thread ID\n");
-        pConv->instance->lastError = DMLERR_INVALIDPARAMETER; /* FIXME: check */
+        WARN("found conv but ain't connected\n");
+        pConv->instance->lastError = DMLERR_NO_CONV_ESTABLISHED;
 	return NULL;
     }
 
@@ -1995,18 +1979,13 @@ BOOL		WDML_PostAck(WDML_CONV* pConv, WDM
 BOOL WINAPI DdeSetUserHandle(HCONV hConv, DWORD id, DWORD hUser)
 {
     WDML_CONV*	pConv;
-    BOOL	ret = TRUE;
 
     TRACE("(%p,%x,%x)\n", hConv, id, hUser);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     pConv = WDML_GetConv(hConv, FALSE);
     if (pConv == NULL)
-    {
-	ret = FALSE;
-	goto theError;
-    }
+	return FALSE;
+
     if (id == QID_SYNC)
     {
 	pConv->hUser = hUser;
@@ -2023,12 +2002,10 @@ BOOL WINAPI DdeSetUserHandle(HCONV hConv
 	else
 	{
 	    pConv->instance->lastError = DMLERR_UNFOUND_QUEUE_ID;
-	    ret = FALSE;
+	    return  FALSE;
 	}
     }
- theError:
-    LeaveCriticalSection(&WDML_CritSect);
-    return ret;
+    return TRUE;
 }
 
 /******************************************************************
@@ -2127,8 +2104,6 @@ UINT WINAPI DdeQueryConvInfo(HCONV hConv
         return 0;
     }
 
-    EnterCriticalSection(&WDML_CritSect);
-
     pConv = WDML_GetConv(hConv, FALSE);
     if (pConv != NULL)
     {
@@ -2145,7 +2120,7 @@ UINT WINAPI DdeQueryConvInfo(HCONV hConv
         }
         ret = 0;
     }
-    LeaveCriticalSection(&WDML_CritSect);
+
     if (ret != 0)
 	memcpy(lpConvInfo, &ci, min((size_t)lpConvInfo->cb, sizeof(ci)));
     return ret;
diff --git a/dlls/user/dde_private.h b/dlls/user/dde_private.h
index fc5656d..f784938 100644
--- a/dlls/user/dde_private.h
+++ b/dlls/user/dde_private.h
@@ -169,8 +169,6 @@ typedef struct tagWDML_INSTANCE
     WDML_LINK*			links[2];	/* active links for this instance (client and server) */
 } WDML_INSTANCE;
 
-extern CRITICAL_SECTION WDML_CritSect;		/* protection for instance list */
-
 /* header for the DDE Data objects */
 typedef struct tagDDE_DATAHANDLE_HEAD
 {
diff --git a/dlls/user/dde_server.c b/dlls/user/dde_server.c
index 5f0f5ee..545b80e 100644
--- a/dlls/user/dde_server.c
+++ b/dlls/user/dde_server.c
@@ -69,17 +69,13 @@ BOOL WINAPI DdePostAdvise(DWORD idInst, 
 
     TRACE("(%d,%p,%p)\n", idInst, hszTopic, hszItem);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     pInstance = WDML_GetInstance(idInst);
 
     if (pInstance == NULL || pInstance->links == NULL)
-    {
-	goto theError;
-    }
+        return FALSE;
 
     atom = WDML_MakeAtomFromHsz(hszItem);
-    if (!atom) goto theError;
+    if (!atom) return FALSE;
 
     /* first compute the number of links which will trigger a message */
     count = 0;
@@ -145,10 +141,9 @@ BOOL WINAPI DdePostAdvise(DWORD idInst, 
 	    }
 	}
     }
-    LeaveCriticalSection(&WDML_CritSect);
     return TRUE;
+
  theError:
-    LeaveCriticalSection(&WDML_CritSect);
     if (atom) GlobalDeleteAtom(atom);
     return FALSE;
 }
@@ -179,8 +174,6 @@ HDDEDATA WINAPI DdeNameService(DWORD idI
 
     TRACE("(%d,%p,%p,%x)\n", idInst, hsz1, hsz2, afCmd);
 
-    EnterCriticalSection(&WDML_CritSect);
-
     /*  First check instance
      */
     pInstance = WDML_GetInstance(idInst);
@@ -188,7 +181,7 @@ HDDEDATA WINAPI DdeNameService(DWORD idI
     {
 	TRACE("Instance not found as initialised\n");
 	/*  Nothing has been initialised - exit now ! can return TRUE since effect is the same */
-	goto theError;
+        return NULL;
     }
 
     if (hsz2 != 0L)
@@ -197,7 +190,7 @@ HDDEDATA WINAPI DdeNameService(DWORD idI
 	 */
 	pInstance->lastError = DMLERR_INVALIDPARAMETER;
 	WARN("Reserved parameter no-zero !!\n");
-	goto theError;
+        return NULL;
     }
     if (hsz1 == 0 && !(afCmd & DNS_UNREGISTER))
     {
@@ -206,7 +199,7 @@ HDDEDATA WINAPI DdeNameService(DWORD idI
 	 */
 	TRACE("General unregister unexpected flags\n");
 	pInstance->lastError = DMLERR_INVALIDPARAMETER;
-	goto theError;
+        return NULL;
     }
 
     switch (afCmd & (DNS_REGISTER | DNS_UNREGISTER))
@@ -217,7 +210,7 @@ HDDEDATA WINAPI DdeNameService(DWORD idI
 	{
 	    ERR("Trying to register already registered service!\n");
 	    pInstance->lastError = DMLERR_DLL_USAGE;
-	    goto theError;
+            return NULL;
 	}
 
 	TRACE("Adding service name\n");
@@ -244,11 +237,9 @@ HDDEDATA WINAPI DdeNameService(DWORD idI
 
 	RegisterClassExW(&wndclass);
 
-	LeaveCriticalSection(&WDML_CritSect);
 	hwndServer = CreateWindowW(szServerNameClass, NULL,
 				   WS_POPUP, 0, 0, 0, 0,
 				   0, 0, 0, 0);
-	EnterCriticalSection(&WDML_CritSect);
 
 	SetWindowLongPtrW(hwndServer, GWL_WDML_INSTANCE, (ULONG_PTR)pInstance);
 	SetWindowLongPtrW(hwndServer, GWL_WDML_SERVER, (ULONG_PTR)pServer);
@@ -285,19 +276,14 @@ HDDEDATA WINAPI DdeNameService(DWORD idI
 	    /*  trying to filter where no service names !!
 	     */
 	    pInstance->lastError = DMLERR_DLL_USAGE;
-	    goto theError;
+            return NULL;
 	}
 	else
 	{
 	    pServer->filterOn = (afCmd & DNS_FILTERON) != 0;
 	}
     }
-    LeaveCriticalSection(&WDML_CritSect);
     return (HDDEDATA)TRUE;
-
- theError:
-    LeaveCriticalSection(&WDML_CritSect);
-    return FALSE;
 }
 
 /******************************************************************
@@ -998,13 +984,11 @@ static LRESULT CALLBACK WDML_ServerConvP
 
     if (iMsg == WM_DESTROY)
     {
-	EnterCriticalSection(&WDML_CritSect);
 	pConv = WDML_GetConvFromWnd(hwndServer);
 	if (pConv && !(pConv->wStatus & ST_TERMINATED))
 	{
 	    WDML_ServerHandleTerminate(pConv, NULL);
 	}
-	LeaveCriticalSection(&WDML_CritSect);
     }
     if (iMsg < WM_DDE_FIRST || iMsg > WM_DDE_LAST)
     {
@@ -1012,25 +996,23 @@ static LRESULT CALLBACK WDML_ServerConvP
                                              DefWindowProcA(hwndServer, iMsg, wParam, lParam);
     }
 
-    EnterCriticalSection(&WDML_CritSect);
-
     pInstance = WDML_GetInstanceFromWnd(hwndServer);
     pConv = WDML_GetConvFromWnd(hwndServer);
 
     if (!pConv)
     {
 	ERR("Got a message (%x) on a not known conversation, dropping request\n", iMsg);
-	goto theError;
+        return 0;
     }
     if (pConv->hwndClient != WIN_GetFullHandle( (HWND)wParam ) || pConv->hwndServer != hwndServer)
     {
 	ERR("mismatch between C/S windows and converstation\n");
-	goto theError;
+        return 0;
     }
     if (pConv->instance != pInstance || pConv->instance == NULL)
     {
 	ERR("mismatch in instances\n");
-	goto theError;
+        return 0;
     }
 
     switch (iMsg)
@@ -1083,7 +1065,6 @@ static LRESULT CALLBACK WDML_ServerConvP
 	    WDML_FreeTransaction(pInstance, pXAct, TRUE);
 	}
     }
- theError:
-    LeaveCriticalSection(&WDML_CritSect);
+
     return 0;
 }
-- 
1.4.2






More information about the wine-patches mailing list