Re: [hatari-devel] Crash when opening Mac preferences dialog

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


Hi,

On 27.8.2023 19.25, Chris Jenkins wrote:
However, SDL GUI gives the returned string back to
Midi_Host_GetPortName() when using non-zero offsets.

And your patch breaks that i.e. Mac GUI would have just the name of the
first MIDI device in the list, not all of them.

Many thanks for spotting and explaining that. I confess I had not grokked
the difference between what the Mac and SDL GUIs are doing.

Would something like attached patch make it clearer?


Please try whether the attached patch fixes the crash.

Yes, your patch fixes the crash.

Thanks, I've pushed it.


	- Eero
From 51cc6a3b707eb73c2500f277916de52ed933b63b Mon Sep 17 00:00:00 2001
From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
Date: Sun, 27 Aug 2023 22:03:14 +0300
Subject: [PATCH 1/2] Use named argument values for MIDI functions

To make clearer what the integer and boolean argument values are for.
---
 src/gui-osx/PrefsController.m |  4 ++--
 src/gui-sdl/dlgDevice.c       | 12 ++++++------
 src/includes/midi.h           | 13 ++++++++++++-
 src/midi.c                    | 18 +++++++++---------
 4 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/src/gui-osx/PrefsController.m b/src/gui-osx/PrefsController.m
index e6c90e84..38f20a2a 100644
--- a/src/gui-osx/PrefsController.m
+++ b/src/gui-osx/PrefsController.m
@@ -575,11 +575,11 @@ char szPath[FILENAME_MAX];
 #ifdef HAVE_PORTMIDI
 	int i = 0;
 	const char* portName = NULL;
-	while ((portName = Midi_Host_GetPortName(portName, +1, true)))
+	while ((portName = Midi_Host_GetPortName(portName, MIDI_NAME_NEXT, MIDI_FOR_INPUT)))
 		[midiInPort addItemWithTitle:[NSString stringWithCString:portName encoding:NSASCIIStringEncoding]];
 	i = 0;
 	portName = NULL;
-	while ((portName = Midi_Host_GetPortName(portName, +1, false)))
+	while ((portName = Midi_Host_GetPortName(portName, MIDI_NAME_NEXT, MIDI_FOR_OUTPUT)))
 		[midiOutPort addItemWithTitle:[NSString stringWithCString:portName encoding:NSASCIIStringEncoding]];
 #endif		// HAVE_PORTMIDI
 }
diff --git a/src/gui-sdl/dlgDevice.c b/src/gui-sdl/dlgDevice.c
index 4c17a440..f80c08c9 100644
--- a/src/gui-sdl/dlgDevice.c
+++ b/src/gui-sdl/dlgDevice.c
@@ -134,10 +134,10 @@ void Dialog_DeviceDlg(void)
 	File_ShrinkName(dlgMidiInName, ConfigureParams.Midi.sMidiInFileName, devicedlg[DEVDLG_MIDIINNAME].w);
 	File_ShrinkName(dlgMidiOutName, ConfigureParams.Midi.sMidiOutFileName, devicedlg[DEVDLG_MIDIOUTNAME].w);
 #else
-	midiInName = Midi_Host_GetPortName(ConfigureParams.Midi.sMidiInPortName, 0, true);
+	midiInName = Midi_Host_GetPortName(ConfigureParams.Midi.sMidiInPortName, MIDI_NAME_FIND, MIDI_FOR_INPUT);
 	File_ShrinkName(dlgMidiInName, midiInName ? midiInName : "Off", devicedlg[DEVDLG_MIDIINNAME].w);
 
-	midiOutName = Midi_Host_GetPortName(ConfigureParams.Midi.sMidiOutPortName, 0, false);
+	midiOutName = Midi_Host_GetPortName(ConfigureParams.Midi.sMidiOutPortName, MIDI_NAME_FIND, MIDI_FOR_OUTPUT);
 	File_ShrinkName(dlgMidiOutName,  midiOutName ? midiOutName : "Off", devicedlg[DEVDLG_MIDIOUTNAME].w);
 #endif
 
@@ -181,12 +181,12 @@ void Dialog_DeviceDlg(void)
 			break;
 #else
 		case DEVDLG_PREVIN:
-			midiInName = Midi_Host_GetPortName(midiInName, -1, true);
+			midiInName = Midi_Host_GetPortName(midiInName, MIDI_NAME_PREV, MIDI_FOR_INPUT);
 			File_ShrinkName(dlgMidiInName, midiInName ? midiInName : "Off",
 					devicedlg[DEVDLG_MIDIINNAME].w);
 			break;
 		case DEVDLG_NEXTIN:
-			name = Midi_Host_GetPortName(midiInName, +1, true);
+			name = Midi_Host_GetPortName(midiInName, MIDI_NAME_NEXT, MIDI_FOR_INPUT);
 			if (name)
 			{
 				midiInName = name;
@@ -195,12 +195,12 @@ void Dialog_DeviceDlg(void)
 			}
 			break;
 		case DEVDLG_PREVOUT:
-			midiOutName = Midi_Host_GetPortName(midiOutName, -1, false);
+			midiOutName = Midi_Host_GetPortName(midiOutName, MIDI_NAME_PREV, MIDI_FOR_OUTPUT);
 			File_ShrinkName(dlgMidiOutName, midiOutName ? midiOutName : "Off",
 					devicedlg[DEVDLG_MIDIOUTNAME].w);
 			break;
 		case DEVDLG_NEXTOUT:
-			name = Midi_Host_GetPortName(midiOutName, +1, false);
+			name = Midi_Host_GetPortName(midiOutName, MIDI_NAME_NEXT, MIDI_FOR_OUTPUT);
 			if (name)
 			{
 				midiOutName = name;
diff --git a/src/includes/midi.h b/src/includes/midi.h
index 48a85699..4e9724c0 100644
--- a/src/includes/midi.h
+++ b/src/includes/midi.h
@@ -19,8 +19,19 @@ extern void Midi_Control_WriteByte(void);
 extern void Midi_Data_WriteByte(void);
 extern void Midi_InterruptHandler_Update(void);
 
+typedef enum {
+	MIDI_FOR_INPUT,
+	MIDI_FOR_OUTPUT
+} midi_dir_t;
+
 #ifdef HAVE_PORTMIDI
-extern const char* Midi_Host_GetPortName(const char *name, int offset, bool forInput);
+typedef enum {
+	MIDI_NAME_PREV = -1,
+	MIDI_NAME_FIND = 0,
+	MIDI_NAME_NEXT = +1
+} midi_name_offset_t;
+
+extern const char* Midi_Host_GetPortName(const char *name, midi_name_offset_t offset, midi_dir_t dir);
 #endif
 
 #endif
diff --git a/src/midi.c b/src/midi.c
index bf4f3607..2fce4757 100644
--- a/src/midi.c
+++ b/src/midi.c
@@ -72,7 +72,7 @@ static FILE *pMidiFhOut = NULL;    /* File handle used for Midi output */
 static PmStream* midiIn  = NULL;	 // current midi input port
 static PmStream* midiOut = NULL;	 // current midi output port
 
-static bool Midi_Host_SwitchPort(const char* portName, bool forInput);
+static bool Midi_Host_SwitchPort(const char* portName, midi_dir_t dir);
 static int Midi_GetDataLength(uint8_t status);
 static int Midi_SplitEvent(PmEvent* midiEvent, uint8_t* msg);
 static PmEvent* Midi_BuildEvent(uint8_t byte);
@@ -433,7 +433,7 @@ static void Midi_Host_Close(void)
  * (i.e. before any port has been selected for the first time),
  * name of the first port in that direction is returned.
  */
-const char* Midi_Host_GetPortName(const char *name, int offset, bool forInput)
+const char* Midi_Host_GetPortName(const char *name, midi_name_offset_t offset, midi_dir_t dir)
 {
 	const PmDeviceInfo* info;
 	const char *prev = NULL;
@@ -450,9 +450,9 @@ const char* Midi_Host_GetPortName(const char *name, int offset, bool forInput)
 		info = Pm_GetDeviceInfo(i);
 		if (!info)
 			continue;
-		if (forInput && !info->input)
+		if (dir == MIDI_FOR_INPUT && !info->input)
 			continue;
-		if (!forInput && info->input)
+		if (dir == MIDI_FOR_OUTPUT && info->input)
 			continue;
 		if (len == 0)
 		{
@@ -487,7 +487,7 @@ const char* Midi_Host_GetPortName(const char *name, int offset, bool forInput)
  * matches beginning of the device name, is used. Returns true for
  * success, false otherwise
  */
-static bool Midi_Host_SwitchPort(const char* portName, bool forInput)
+static bool Midi_Host_SwitchPort(const char* portName, midi_dir_t dir)
 {
 	int i, prefixmatch, len, count;
 	bool err;
@@ -509,9 +509,9 @@ static bool Midi_Host_SwitchPort(const char* portName, bool forInput)
 		const PmDeviceInfo* info = Pm_GetDeviceInfo(i);
 		if (!info)
 			continue;
-		if (forInput && !info->input)
+		if (dir == MIDI_FOR_INPUT && !info->input)
 			continue;
-		if (!forInput && info->input)
+		if (dir == MIDI_FOR_OUTPUT && info->input)
 			continue;
 		if (!strcmp(info->name, portName))
 			break;
@@ -524,12 +524,12 @@ static bool Midi_Host_SwitchPort(const char* portName, bool forInput)
 	if (i >= count)
 	{
 		LOG_TRACE(TRACE_MIDI, "MIDI: no %s ports matching '%s'\n",
-			  forInput ? "input" : "output", portName);
+			  dir == MIDI_FOR_INPUT ? "input" : "output", portName);
 		return false;
 	}
 
 	// -- close current port in any case, then try open new one
-	if (forInput == true)
+	if (dir == MIDI_FOR_INPUT)
 	{
 		if (midiIn) {
 			Pm_Close(midiIn);
-- 
2.39.2



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/