[PATCH 5/5 v2] wineoss: Move MODM_OPEN and MODM_CLOSE to the unixlib.

[Andrew Eikum]

From: Huw Davies <huw(a)codeweavers.com>

They are both moved together because of the memory
allocated for "lpExtra".

Signed-off-by: Huw Davies <huw(a)codeweavers.com>
Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com>
---
 dlls/wineoss.drv/midi.c    | 180 ++++++----------------------------
 dlls/wineoss.drv/oss.c     |   2 +-
 dlls/wineoss.drv/ossmidi.c | 191 +++++++++++++++++++++++++++++++++++--
 dlls/wineoss.drv/unixlib.h |  28 +++++-
 4 files changed, 235 insertions(+), 166 deletions(-)

diff --git a/dlls/wineoss.drv/midi.c b/dlls/wineoss.drv/midi.c
index 7d21dd32ae8..d66e926655d 100644
--- a/dlls/wineoss.drv/midi.c
+++ b/dlls/wineoss.drv/midi.c
@@ -149,6 +149,15 @@ static LRESULT OSS_MidiExit(void)
     return 0;
 }
 
+static void notify_client(struct notify_context *notify)
+{
+    TRACE("dev_id = %d msg = %d param1 = %04lX param2 = %04lX\n",
+          notify->dev_id, notify->msg, notify->param_1, notify->param_2);
+
+    DriverCallback(notify->callback, notify->flags, notify->device, notify->msg,
+                   notify->instance, notify->param_1, notify->param_2);
+}
+
 /**************************************************************************
  * 			MIDI_NotifyClient			[internal]
  */
@@ -164,8 +173,6 @@ static void MIDI_NotifyClient(UINT wDevID, WORD wMsg,
 	  wDevID, wMsg, dwParam1, dwParam2);
 
     switch (wMsg) {
-    case MOM_OPEN:
-    case MOM_CLOSE:
     case MOM_DONE:
     case MOM_POSITIONCB:
 	if (wDevID > MODM_NumDevs) return;
@@ -723,20 +730,6 @@ typedef struct sFMextra {
      */
 } sFMextra;
 
-/**************************************************************************
- * 			modFMLoad				[internal]
- */
-static int modFMLoad(WORD dev, int fd)
-{
-    struct midi_out_fm_load_params params;
-
-    params.dev_id = dev;
-    params.fd = fd;
-    OSS_CALL(midi_out_fm_load, ¶ms);
-
-    return params.ret;
-}
-
 #define		IS_DRUM_CHANNEL(_xtra, _chn)	((_xtra)->drumSetMask & (1 << (_chn)))
 
 /**************************************************************************
@@ -754,132 +747,6 @@ static DWORD modGetDevCaps(WORD wDevID, LPMIDIOUTCAPSW lpCaps, DWORD dwSize)
     return MMSYSERR_NOERROR;
 }
 
-/**************************************************************************
- * 			modOpen					[internal]
- */
-static DWORD modOpen(WORD wDevID, LPMIDIOPENDESC lpDesc, DWORD dwFlags)
-{
-    int fd = -1;
-
-    TRACE("(%04X, %p, %08X);\n", wDevID, lpDesc, dwFlags);
-    if (lpDesc == NULL) {
-	WARN("Invalid Parameter !\n");
-	return MMSYSERR_INVALPARAM;
-    }
-    if (wDevID >= MODM_NumDevs) {
-	TRACE("MAX_MIDIOUTDRV reached !\n");
-	return MMSYSERR_BADDEVICEID;
-    }
-    if (MidiOutDev[wDevID].midiDesc.hMidi != 0) {
-	WARN("device already open !\n");
-	return MMSYSERR_ALLOCATED;
-    }
-    if (!MidiOutDev[wDevID].bEnabled) {
-	WARN("device disabled !\n");
-	return MIDIERR_NODEVICE;
-    }
-    if ((dwFlags & ~CALLBACK_TYPEMASK) != 0) {
-	WARN("bad dwFlags\n");
-	return MMSYSERR_INVALFLAG;
-    }
-
-    MidiOutDev[wDevID].lpExtra = 0;
-
-    switch (MidiOutDev[wDevID].caps.wTechnology) {
-    case MOD_FMSYNTH:
-	{
-	    void*	extra;
-
-            extra = HeapAlloc(GetProcessHeap(), 0,
-                              offsetof(struct sFMextra, voice[MidiOutDev[wDevID].caps.wVoices]));
-
-	    if (extra == 0) {
-		WARN("can't alloc extra data !\n");
-		return MMSYSERR_NOMEM;
-	    }
-	    MidiOutDev[wDevID].lpExtra = extra;
-	    fd = midiOpenSeq();
-	    if (fd < 0) {
-		MidiOutDev[wDevID].lpExtra = 0;
-		HeapFree(GetProcessHeap(), 0, extra);
-		return MMSYSERR_ERROR;
-	    }
-	    if (modFMLoad(wDevID, fd) < 0) {
-		midiCloseSeq(fd);
-		MidiOutDev[wDevID].lpExtra = 0;
-		HeapFree(GetProcessHeap(), 0, extra);
-		return MMSYSERR_ERROR;
-	    }
-	    OSS_CALL(midi_out_fm_reset, (void *)(UINT_PTR)wDevID);
-	}
-	break;
-    case MOD_MIDIPORT:
-    case MOD_SYNTH:
-	fd = midiOpenSeq();
-	if (fd < 0) {
-	    return MMSYSERR_ALLOCATED;
-	}
-	break;
-    default:
-	WARN("Technology not supported (yet) %d !\n",
-	     MidiOutDev[wDevID].caps.wTechnology);
-	return MMSYSERR_NOTENABLED;
-    }
-
-    MidiOutDev[wDevID].wFlags = HIWORD(dwFlags & CALLBACK_TYPEMASK);
-
-    MidiOutDev[wDevID].lpQueueHdr = NULL;
-    MidiOutDev[wDevID].midiDesc = *lpDesc;
-    MidiOutDev[wDevID].fd = fd;
-
-    MIDI_NotifyClient(wDevID, MOM_OPEN, 0L, 0L);
-    TRACE("Successful !\n");
-    return MMSYSERR_NOERROR;
-}
-
-
-/**************************************************************************
- * 			modClose				[internal]
- */
-static DWORD modClose(WORD wDevID)
-{
-    int	ret = MMSYSERR_NOERROR;
-
-    TRACE("(%04X);\n", wDevID);
-
-    if (MidiOutDev[wDevID].midiDesc.hMidi == 0) {
-	WARN("device not opened !\n");
-	return MMSYSERR_ERROR;
-    }
-    /* FIXME: should test that no pending buffer is still in the queue for
-     * playing */
-
-    if (MidiOutDev[wDevID].fd == -1) {
-	WARN("can't close !\n");
-	return MMSYSERR_ERROR;
-    }
-
-    switch (MidiOutDev[wDevID].caps.wTechnology) {
-    case MOD_FMSYNTH:
-    case MOD_SYNTH:
-    case MOD_MIDIPORT:
-	midiCloseSeq(MidiOutDev[wDevID].fd);
-	break;
-    default:
-	WARN("Technology not supported (yet) %d !\n",
-	     MidiOutDev[wDevID].caps.wTechnology);
-	return MMSYSERR_NOTENABLED;
-    }
-
-    HeapFree(GetProcessHeap(), 0, MidiOutDev[wDevID].lpExtra);
-    MidiOutDev[wDevID].lpExtra = 0;
-    MidiOutDev[wDevID].fd = -1;
-
-    MIDI_NotifyClient(wDevID, MOM_CLOSE, 0L, 0L);
-    MidiOutDev[wDevID].midiDesc.hMidi = 0;
-    return ret;
-}
-
 /**************************************************************************
  * 			modData					[internal]
  */
@@ -1378,6 +1245,10 @@ DWORD WINAPI OSS_midMessage(UINT wDevID, UINT wMsg, DWORD_PTR dwUser,
 DWORD WINAPI OSS_modMessage(UINT wDevID, UINT wMsg, DWORD_PTR dwUser,
 			    DWORD_PTR dwParam1, DWORD_PTR dwParam2)
 {
+    struct midi_out_message_params params;
+    struct notify_context notify;
+    UINT err;
+
     TRACE("(%04X, %04X, %08lX, %08lX, %08lX);\n",
 	  wDevID, wMsg, dwUser, dwParam1, dwParam2);
 
@@ -1386,14 +1257,6 @@ DWORD WINAPI OSS_modMessage(UINT wDevID, UINT wMsg, DWORD_PTR dwUser,
         return OSS_MidiInit();
     case DRVM_EXIT:
         return OSS_MidiExit();
-    case DRVM_ENABLE:
-    case DRVM_DISABLE:
-	/* FIXME: Pretend this is supported */
-	return 0;
-    case MODM_OPEN:
-	return modOpen(wDevID, (LPMIDIOPENDESC)dwParam1, dwParam2);
-    case MODM_CLOSE:
-	return modClose(wDevID);
     case MODM_DATA:
 	return modData(wDevID, dwParam1);
     case MODM_LONGDATA:
@@ -1412,10 +1275,21 @@ DWORD WINAPI OSS_modMessage(UINT wDevID, UINT wMsg, DWORD_PTR dwUser,
 	return 0;
     case MODM_RESET:
 	return modReset(wDevID);
-    default:
-	TRACE("Unsupported message\n");
     }
-    return MMSYSERR_NOTSUPPORTED;
+
+    params.dev_id = wDevID;
+    params.msg = wMsg;
+    params.user = dwUser;
+    params.param_1 = dwParam1;
+    params.param_2 = dwParam2;
+    params.err = &err;
+    params.notify = ¬ify;
+
+    OSS_CALL(midi_out_message, ¶ms);
+
+    if (!err && notify.send_notify) notify_client(¬ify);
+
+    return err;
 }
 
 /**************************************************************************
diff --git a/dlls/wineoss.drv/oss.c b/dlls/wineoss.drv/oss.c
index 902685c11b8..1bd5ddb5762 100644
--- a/dlls/wineoss.drv/oss.c
+++ b/dlls/wineoss.drv/oss.c
@@ -1406,8 +1406,8 @@ unixlib_entry_t __wine_unix_call_funcs[] =
     set_event_handle,
     is_started,
     midi_init,
+    midi_out_message,
 
     midi_seq_open,
-    midi_out_fm_load,
     midi_out_fm_reset,
 };
diff --git a/dlls/wineoss.drv/ossmidi.c b/dlls/wineoss.drv/ossmidi.c
index dcafbe609d9..613b070fde5 100644
--- a/dlls/wineoss.drv/ossmidi.c
+++ b/dlls/wineoss.drv/ossmidi.c
@@ -424,11 +424,8 @@ void seqbuf_dump(void)
 extern const unsigned char midiFMInstrumentPatches[16 * 128];
 extern const unsigned char midiFMDrumsPatches[16 * 128];
 
-NTSTATUS midi_out_fm_load(void *args)
+static int midi_out_fm_load(WORD dev_id, int fd)
 {
-    struct midi_out_fm_load_params *params = args;
-    WORD dev_id = params->dev_id;
-    int fd = params->fd;
     struct sbi_instrument sbi;
     int i;
 
@@ -444,8 +441,7 @@ NTSTATUS midi_out_fm_load(void *args)
         if (write(fd, &sbi, sizeof(sbi)) == -1)
         {
             WARN("Couldn't write patch for instrument %d, errno %d (%s)!\n", sbi.channel, errno, strerror(errno));
-            params->ret = -1;
-            return STATUS_SUCCESS;
+            return -1;
         }
     }
     for (i = 0; i < 128; i++)
@@ -456,12 +452,10 @@ NTSTATUS midi_out_fm_load(void *args)
         if (write(fd, &sbi, sizeof(sbi)) == -1)
         {
             WARN("Couldn't write patch for drum %d, errno %d (%s)!\n", sbi.channel, errno, strerror(errno));
-            params->ret = -1;
-            return STATUS_SUCCESS;
+            return -1;
         }
     }
-    params->ret = 0;
-    return STATUS_SUCCESS;
+    return 0;
 }
 
 NTSTATUS midi_out_fm_reset(void *args)
@@ -501,3 +495,180 @@ NTSTATUS midi_out_fm_reset(void *args)
 
     return STATUS_SUCCESS;
 }
+
+static void set_out_notify(struct notify_context *notify, struct midi_dest *dest, WORD dev_id, WORD msg,
+                           UINT_PTR param_1, UINT_PTR param_2)
+{
+    notify->send_notify = TRUE;
+    notify->dev_id = dev_id;
+    notify->msg = msg;
+    notify->param_1 = param_1;
+    notify->param_2 = param_2;
+    notify->callback = dest->midiDesc.dwCallback;
+    notify->flags = dest->wFlags;
+    notify->device = dest->midiDesc.hMidi;
+    notify->instance = dest->midiDesc.dwInstance;
+}
+
+static UINT midi_out_open(WORD dev_id, MIDIOPENDESC *midi_desc, UINT flags, struct notify_context *notify)
+{
+    struct midi_dest *dest;
+    int fd = -1;
+
+    TRACE("(%04X, %p, %08X);\n", dev_id, midi_desc, flags);
+    if (midi_desc == NULL)
+    {
+        WARN("Invalid Parameter !\n");
+        return MMSYSERR_INVALPARAM;
+    }
+    if (dev_id >= num_dests)
+    {
+        TRACE("MAX_MIDIOUTDRV reached !\n");
+        return MMSYSERR_BADDEVICEID;
+    }
+    dest = dests + dev_id;
+    if (dest->midiDesc.hMidi != 0)
+    {
+        WARN("device already open !\n");
+        return MMSYSERR_ALLOCATED;
+    }
+    if (!dest->bEnabled)
+    {
+        WARN("device disabled !\n");
+        return MIDIERR_NODEVICE;
+    }
+    if ((flags & ~CALLBACK_TYPEMASK) != 0)
+    {
+        WARN("bad flags\n");
+        return MMSYSERR_INVALFLAG;
+    }
+
+    dest->lpExtra = NULL;
+
+    switch (dest->caps.wTechnology)
+    {
+    case MOD_FMSYNTH:
+    {
+        void *extra;
+
+        extra = malloc(offsetof(struct sFMextra, voice[dest->caps.wVoices]));
+        if (!extra)
+        {
+            WARN("can't alloc extra data !\n");
+            return MMSYSERR_NOMEM;
+        }
+        dest->lpExtra = extra;
+        fd = seq_open();
+        if (fd < 0)
+        {
+            dest->lpExtra = NULL;
+            free(extra);
+            return MMSYSERR_ERROR;
+        }
+        if (midi_out_fm_load(dev_id, fd) < 0)
+        {
+            seq_close(fd);
+            dest->lpExtra = NULL;
+            free(extra);
+            return MMSYSERR_ERROR;
+        }
+        midi_out_fm_reset((void *)(UINT_PTR)dev_id);
+        break;
+    }
+    case MOD_MIDIPORT:
+    case MOD_SYNTH:
+        fd = seq_open();
+        if (fd < 0)
+            return MMSYSERR_ALLOCATED;
+        break;
+    default:
+        WARN("Technology not supported (yet) %d !\n", dest->caps.wTechnology);
+        return MMSYSERR_NOTENABLED;
+    }
+
+    dest->wFlags = HIWORD(flags & CALLBACK_TYPEMASK);
+
+    dest->lpQueueHdr= NULL;
+    dest->midiDesc = *midi_desc;
+    dest->fd = fd;
+
+    set_out_notify(notify, dest, dev_id, MOM_OPEN, 0, 0);
+    TRACE("Successful !\n");
+    return MMSYSERR_NOERROR;
+}
+
+static UINT midi_out_close(WORD dev_id, struct notify_context *notify)
+{
+    struct midi_dest *dest;
+
+    TRACE("(%04X);\n", dev_id);
+
+    if (dev_id >= num_dests)
+    {
+        TRACE("MAX_MIDIOUTDRV reached !\n");
+        return MMSYSERR_BADDEVICEID;
+    }
+    dest = dests + dev_id;
+
+    if (dest->midiDesc.hMidi == 0)
+    {
+        WARN("device not opened !\n");
+        return MMSYSERR_ERROR;
+    }
+    /* FIXME: should test that no pending buffer is still in the queue for
+     * playing */
+
+    if (dest->fd == -1)
+    {
+        WARN("can't close !\n");
+        return MMSYSERR_ERROR;
+    }
+
+    switch (dest->caps.wTechnology)
+    {
+    case MOD_FMSYNTH:
+    case MOD_SYNTH:
+    case MOD_MIDIPORT:
+        seq_close(dest->fd);
+        break;
+    default:
+        WARN("Technology not supported (yet) %d !\n", dest->caps.wTechnology);
+        return MMSYSERR_NOTENABLED;
+    }
+
+    free(dest->lpExtra);
+    dest->lpExtra = NULL;
+    dest->fd = -1;
+
+    set_out_notify(notify, dest, dev_id, MOM_CLOSE, 0, 0);
+    dest->midiDesc.hMidi = 0;
+    return MMSYSERR_NOERROR;
+}
+
+
+NTSTATUS midi_out_message(void *args)
+{
+    struct midi_out_message_params *params = args;
+
+    params->notify->send_notify = FALSE;
+
+    switch (params->msg)
+    {
+    case DRVM_ENABLE:
+    case DRVM_DISABLE:
+        /* FIXME: Pretend this is supported */
+        *params->err = MMSYSERR_NOERROR;
+        break;
+    case MODM_OPEN:
+        *params->err = midi_out_open(params->dev_id, (MIDIOPENDESC *)params->param_1, params->param_2, params->notify);
+        break;
+    case MODM_CLOSE:
+        *params->err = midi_out_close(params->dev_id, params->notify);
+        break;
+    default:
+        TRACE("Unsupported message\n");
+        *params->err = MMSYSERR_NOTSUPPORTED;
+    }
+
+    return STATUS_SUCCESS;
+}
diff --git a/dlls/wineoss.drv/unixlib.h b/dlls/wineoss.drv/unixlib.h
index ecca092d540..75506d445bc 100644
--- a/dlls/wineoss.drv/unixlib.h
+++ b/dlls/wineoss.drv/unixlib.h
@@ -246,6 +246,30 @@ struct midi_init_params
     struct midi_src *srcs;
 };
 
+struct notify_context
+{
+    BOOL send_notify;
+    WORD dev_id;
+    WORD msg;
+    UINT_PTR param_1;
+    UINT_PTR param_2;
+    UINT_PTR callback;
+    UINT flags;
+    HANDLE device;
+    UINT_PTR instance;
+};
+
+struct midi_out_message_params
+{
+    UINT dev_id;
+    UINT msg;
+    UINT_PTR user;
+    UINT_PTR param_1;
+    UINT_PTR param_2;
+    UINT *err;
+    struct notify_context *notify;
+};
+
 struct midi_seq_open_params
 {
     int close;
@@ -285,15 +309,15 @@ enum oss_funcs
     oss_set_event_handle,
     oss_is_started,
     oss_midi_init,
+    oss_midi_out_message,
 
     oss_midi_seq_open, /* temporary */
-    oss_midi_out_fm_load,
     oss_midi_out_fm_reset,
 };
 
 NTSTATUS midi_init(void *args) DECLSPEC_HIDDEN;
+NTSTATUS midi_out_message(void *args) DECLSPEC_HIDDEN;
 NTSTATUS midi_seq_open(void *args) DECLSPEC_HIDDEN;
-NTSTATUS midi_out_fm_load(void *args) DECLSPEC_HIDDEN;
 NTSTATUS midi_out_fm_reset(void *args) DECLSPEC_HIDDEN;
 
 extern unixlib_handle_t oss_handle;
-- 
2.36.0

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=112953

Your paranoid android.


=== debian11 (build log) ===

/home/winetest/tools/testbot/var/wine-win32/../wine/dlls/wineoss.drv/ossmidi.c:433: undefined reference to `midiFMInstrumentPatches'
/usr/bin/ld: /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/wineoss.drv/ossmidi.c:454: undefined reference to `midiFMDrumsPatches'
collect2: error: ld returned 1 exit status
Task: The win32 Wine build failed

=== debian11 (build log) ===

/home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/wineoss.drv/ossmidi.c:433: undefined reference to `midiFMInstrumentPatches'
/usr/bin/ld: /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/wineoss.drv/ossmidi.c:436: undefined reference to `midiFMDrumsPatches'
collect2: error: ld returned 1 exit status
Task: The wow64 Wine build failed