Re: Do not free the header while unmapping the xxxx_UNPREPARE messages
Dmitry Timoshkov wrote:
Hello,
this patch is aimed to fix crashes in the old 16-bit game, which calls waveOutUnprepareHeader16 for busy headers. MMDRV_Message in that case returns WAVERR_STILLPLAYING, but it's not stopping MMDRV_WaveOut_UnMap16To32A() from freeing the header. Slightly later game calls waveOutReset16() which crashes. In that case the duty of freeing the header should be moved to a higher level. I have generalized this approach to all similar cases. I'd rather see the fix being made completely in message16.c by adding the return value to the unmapping functions IMO, it better encapsulates all the 32<=>16 message mappings, and it's bad to spread this across several compilation unit A+ -- Eric Pouech
"Eric Pouech" <eric.pouech(a)wanadoo.fr> wrote:
I'd rather see the fix being made completely in message16.c by adding the return value to the unmapping functions
This approach doesn't look clean to me.
IMO, it better encapsulates all the 32<=>16 message mappings, and it's bad to spread this across several compilation unit
Freeing the header while unmapping the 16/32-bit message is an alien task for mapping functions IMO. -- Dmitry.
Freeing the header while unmapping the 16/32-bit message is an alien task for mapping functions IMO.
since the buffer (the 16 bit segmented to be used instead of the 32bit linear) has been allocated in the mapping function, I do think it's a bad idea to free it outside of the unmapping function A+ -- Eric Pouech
"Eric Pouech" <eric.pouech(a)wanadoo.fr> wrote:
Freeing the header while unmapping the 16/32-bit message is an alien task for mapping functions IMO.
since the buffer (the 16 bit segmented to be used instead of the 32bit linear) has been allocated in the mapping function, I do think it's a bad idea to free it outside of the unmapping function
It seems to me that it's an unnecessary complexity to introduce extra parameter for unmapping function exclusively for xxxx_UNPREPARE messages. Could you please show what you have in mind for the single mapping/unmapping function pair? -- Dmitry.
It seems to me that it's an unnecessary complexity to introduce extra parameter for unmapping function exclusively for xxxx_UNPREPARE messages. Could you please show what you have in mind for the single mapping/unmapping function pair?
yes, we need to add the result of the call to the unmapping function something like this would do A+ -- Eric Pouech cvs diff: Diffing . Index: lolvldrv.c =================================================================== RCS file: /home/cvs/cvsroot/wine/wine/dlls/winmm/lolvldrv.c,v retrieving revision 1.41 diff -u -r1.41 lolvldrv.c --- lolvldrv.c 23 Dec 2002 02:05:30 -0000 1.41 +++ lolvldrv.c 27 Dec 2002 15:53:41 -0000 @@ -39,9 +39,9 @@ LPSTR typestr; /* name (for debugging) */ BOOL bSupportMapper; /* if type is allowed to support mapper */ MMDRV_MAPFUNC Map16To32A; /* those are function pointers to handle */ - MMDRV_MAPFUNC UnMap16To32A; /* the parameter conversion (16 vs 32 bit) */ + MMDRV_UNMAPFUNC UnMap16To32A; /* the parameter conversion (16 vs 32 bit) */ MMDRV_MAPFUNC Map32ATo16; /* when hi-func (in mmsystem or winmm) and */ - MMDRV_MAPFUNC UnMap32ATo16; /* low-func (in .drv) do not match */ + MMDRV_UNMAPFUNC UnMap32ATo16; /* low-func (in .drv) do not match */ LPDRVCALLBACK Callback; /* handles callback for a specified type */ /* those attributes reflect the loaded/current situation for the type */ UINT wMaxId; /* number of loaded devices (sum across all loaded drivers */ @@ -74,8 +74,8 @@ * */ void MMDRV_InstallMap(unsigned int drv, - MMDRV_MAPFUNC mp1632, MMDRV_MAPFUNC um1632, - MMDRV_MAPFUNC mp3216, MMDRV_MAPFUNC um3216, + MMDRV_MAPFUNC mp1632, MMDRV_UNMAPFUNC um1632, + MMDRV_MAPFUNC mp3216, MMDRV_UNMAPFUNC um3216, LPDRVCALLBACK cb) { assert(drv < MMDRV_MAX); @@ -234,7 +234,7 @@ dwParam1, dwParam2); TRACE("=> %lu\n", ret); if (map == WINMM_MAP_OKMEM) - llType->UnMap16To32A(wMsg, &mld->dwDriverInstance, &dwParam1, &dwParam2); + llType->UnMap16To32A(wMsg, &mld->dwDriverInstance, &dwParam1, &dwParam2, ret); break; default: FIXME("NIY\n"); @@ -264,7 +264,7 @@ dwParam1, dwParam2); TRACE("=> %lu\n", ret); if (map == WINMM_MAP_OKMEM) - llType->UnMap32ATo16(wMsg, &mld->dwDriverInstance, &dwParam1, &dwParam2); + llType->UnMap32ATo16(wMsg, &mld->dwDriverInstance, &dwParam1, &dwParam2, ret); break; default: FIXME("NIY\n"); Index: message16.c =================================================================== RCS file: /home/cvs/cvsroot/wine/wine/dlls/winmm/message16.c,v retrieving revision 1.6 diff -u -r1.6 message16.c --- message16.c 23 Dec 2002 02:05:30 -0000 1.6 +++ message16.c 27 Dec 2002 15:57:26 -0000 @@ -70,7 +70,7 @@ /************************************************************************** * MMDRV_Aux_UnMap16To32A [internal] */ -static WINMM_MapType MMDRV_Aux_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_Aux_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { return WINMM_MAP_MSGERROR; } @@ -86,7 +86,7 @@ /************************************************************************** * MMDRV_Aux_UnMap32ATo16 [internal] */ -static WINMM_MapType MMDRV_Aux_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_Aux_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { #if 0 case AUXDM_GETDEVCAPS: @@ -126,7 +126,7 @@ /************************************************************************** * MMDRV_Mixer_UnMap16To32A [internal] */ -static WINMM_MapType MMDRV_Mixer_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_Mixer_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { #if 0 MIXERCAPSA micA; @@ -156,7 +156,7 @@ /************************************************************************** * MMDRV_Mixer_UnMap32ATo16 [internal] */ -static WINMM_MapType MMDRV_Mixer_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_Mixer_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { return WINMM_MAP_MSGERROR; } @@ -187,7 +187,7 @@ /************************************************************************** * MMDRV_MidiIn_UnMap16To32A [internal] */ -static WINMM_MapType MMDRV_MidiIn_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_MidiIn_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { return WINMM_MAP_MSGERROR; } @@ -203,7 +203,7 @@ /************************************************************************** * MMDRV_MidiIn_UnMap32ATo16 [internal] */ -static WINMM_MapType MMDRV_MidiIn_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_MidiIn_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { return WINMM_MAP_MSGERROR; } @@ -360,7 +360,7 @@ /************************************************************************** * MMDRV_MidiOut_UnMap16To32A [internal] */ -static WINMM_MapType MMDRV_MidiOut_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_MidiOut_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { WINMM_MapType ret = WINMM_MAP_MSGERROR; @@ -411,7 +411,7 @@ if (mh16->reserved >= sizeof(MIDIHDR)) mh16->dwOffset = mh32->dwOffset; - if (wMsg == MODM_UNPREPARE) { + if (wMsg == MODM_UNPREPARE && fn_ret == MMSYSERR_NOERROR) { HeapFree(GetProcessHeap(), 0, (LPSTR)mh32 - sizeof(LPMIDIHDR)); mh16->lpNext = 0; } @@ -571,7 +571,7 @@ /************************************************************************** * MMDRV_MidiOut_UnMap32ATo16 [internal] */ -static WINMM_MapType MMDRV_MidiOut_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_MidiOut_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { WINMM_MapType ret = WINMM_MAP_MSGERROR; @@ -617,7 +617,7 @@ mh32->dwUser = mh16->dwUser; mh32->dwFlags = mh16->dwFlags; - if (wMsg == MODM_UNPREPARE) { + if (wMsg == MODM_UNPREPARE && fn_ret == MMSYSERR_NOERROR) { HeapFree( GetProcessHeap(), 0, ptr ); mh32->lpNext = 0; } @@ -805,7 +805,7 @@ /************************************************************************** * MMDRV_WaveIn_UnMap16To32A [internal] */ -static WINMM_MapType MMDRV_WaveIn_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_WaveIn_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { WINMM_MapType ret = WINMM_MAP_MSGERROR; @@ -860,7 +860,7 @@ wh16->dwFlags = wh32->dwFlags; wh16->dwLoops = wh32->dwLoops; - if (wMsg == WIDM_UNPREPARE) { + if (wMsg == WIDM_UNPREPARE && fn_ret == MMSYSERR_NOERROR) { HeapFree(GetProcessHeap(), 0, (LPSTR)wh32 - sizeof(LPWAVEHDR)); wh16->lpNext = 0; } @@ -1047,7 +1047,7 @@ /************************************************************************** * MMDRV_WaveIn_UnMap32ATo16 [internal] */ -static WINMM_MapType MMDRV_WaveIn_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_WaveIn_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { WINMM_MapType ret = WINMM_MAP_MSGERROR; @@ -1089,7 +1089,7 @@ wh32->dwLoops = wh16->dwLoops; UnMapLS( *lpParam1 ); - if (wMsg == WIDM_UNPREPARE) { + if (wMsg == WIDM_UNPREPARE && fn_ret == MMSYSERR_NOERROR) { HeapFree( GetProcessHeap(), 0, ptr ); wh32->lpNext = 0; } @@ -1303,7 +1303,7 @@ /************************************************************************** * MMDRV_WaveOut_UnMap16To32A [internal] */ -static WINMM_MapType MMDRV_WaveOut_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_WaveOut_UnMap16To32A(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { WINMM_MapType ret = WINMM_MAP_MSGERROR; @@ -1369,7 +1369,7 @@ wh16->dwFlags = wh32->dwFlags; wh16->dwLoops = wh32->dwLoops; - if (wMsg == WODM_UNPREPARE) { + if (wMsg == WODM_UNPREPARE && fn_ret == MMSYSERR_NOERROR) { HeapFree(GetProcessHeap(), 0, (LPSTR)wh32 - sizeof(LPWAVEHDR)); wh16->lpNext = 0; } @@ -1575,7 +1575,7 @@ /************************************************************************** * MMDRV_WaveOut_UnMap32ATo16 [internal] */ -static WINMM_MapType MMDRV_WaveOut_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2) +static WINMM_MapType MMDRV_WaveOut_UnMap32ATo16(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT fn_ret) { WINMM_MapType ret; @@ -1659,7 +1659,7 @@ wh32->dwLoops = wh16->dwLoops; UnMapLS( *lpParam1 ); - if (wMsg == WODM_UNPREPARE) { + if (wMsg == WODM_UNPREPARE && fn_ret == MMSYSERR_NOERROR) { HeapFree( GetProcessHeap(), 0, ptr ); wh32->lpNext = 0; } Index: winemm.h =================================================================== RCS file: /home/cvs/cvsroot/wine/wine/dlls/winmm/winemm.h,v retrieving revision 1.42 diff -u -r1.42 winemm.h --- winemm.h 11 Nov 2002 19:53:01 -0000 1.42 +++ winemm.h 27 Dec 2002 15:57:36 -0000 @@ -225,6 +225,7 @@ typedef LONG (*MCIPROC16)(DWORD, HDRVR16, WORD, DWORD, DWORD); typedef LONG (*MCIPROC)(DWORD, HDRVR, DWORD, DWORD, DWORD); typedef WINMM_MapType (*MMDRV_MAPFUNC)(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2); +typedef WINMM_MapType (*MMDRV_UNMAPFUNC)(UINT wMsg, LPDWORD lpdwUser, LPDWORD lpParam1, LPDWORD lpParam2, MMRESULT ret); LPWINE_DRIVER DRIVER_FindFromHDrvr(HDRVR hDrvr); BOOL DRIVER_GetLibName(LPCSTR keyName, LPCSTR sectName, LPSTR buf, int sz); @@ -244,8 +245,8 @@ DWORD MMDRV_Message(LPWINE_MLD mld, WORD wMsg, DWORD dwParam1, DWORD dwParam2, BOOL bFrom32); UINT MMDRV_PhysicalFeatures(LPWINE_MLD mld, UINT uMsg, DWORD dwParam1, DWORD dwParam2); BOOL MMDRV_Is32(unsigned int); -void MMDRV_InstallMap(unsigned int, MMDRV_MAPFUNC, MMDRV_MAPFUNC, - MMDRV_MAPFUNC, MMDRV_MAPFUNC, LPDRVCALLBACK); +void MMDRV_InstallMap(unsigned int, MMDRV_MAPFUNC, MMDRV_UNMAPFUNC, + MMDRV_MAPFUNC, MMDRV_UNMAPFUNC, LPDRVCALLBACK); BOOL MCI_Init(void); WINE_MCIDRIVER* MCI_GetDriver(UINT16 uDevID);
"Eric Pouech" <eric.pouech(a)wanadoo.fr> wrote:
something like this would do
Of course your patch works too. But I thought that you have something more sophisticated in mind. Nevertheless, Eric, since you are the author of that code I leave the decision what approach is better to you. -- Dmitry.
participants (2)
-
Dmitry Timoshkov -
Eric Pouech