Re: [PATCH] midimap: Add warning for user in case midi won't work

[Matteo Bruni]

2017-09-23 22:52 GMT+02:00 Fabian Maurer :
> When no midi port or only the midi-through port
> is found, we can inform the user that midi
> won't work as expected.
> Signed-off-by: Fabian Maurer
>
> dlls/midimap/midimap.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/dlls/midimap/midimap.c b/dlls/midimap/midimap.c
> index 64b4dc8546..c80834515b 100644
> --- a/dlls/midimap/midimap.c
> +++ b/dlls/midimap/midimap.c
> @@ -553,6 +553,7 @@ static LRESULT MIDIMAP_drvOpen(void)
>  {
>      MIDIOUTCAPSW       moc;
>      unsigned           dev, i;
> -
>
> WCHAR               throughportW[] = {'M','i','d','i',' ','T','h','r','o','u','g','h',0};
>
> if (midiOutPorts)
>    return 0;
>
> @@ -578,6 +579,13 @@ static LRESULT MIDIMAP_drvOpen(void)
>         }
>      }
> - if(numMidiOutPorts == 0)
> -
>    MESSAGE("Midi sound output won't work - No midi port found.\n");
> - else if(numMidiOutPorts == 1 && midiOutPorts[0].loaded == -1)
> -
>    MESSAGE("Midi sound output won't work - No working midi port found.\n");
> - else if(numMidiOutPorts == 1 && strncmpW(midiOutPorts[0].name, throughportW, strlenW(throughportW)) == 0)
> -
>    MESSAGE("Midi sound output probably won't work - Only 'midi through' port found. Make sure your setup is correct.\n");
> - return 1;
>
> }

Since you have to resend anyway...

The string comparison doesn't look correct to me, I think that breaks
if the midi port name has a prefix matching the throughportW string. I
guess it's not going to be a problem in practice but it seems better
to do it right anyway.
There should be a space between 'if' and the '(', like elsewhere in
the function and source file.

[PATCH] midimap: Add warning for user in case midi won't work

[Fabian Maurer]

Thanks, you mean I should check whether the whole string equals "midi through 
port" or what it is?

Regards,
Fabian Maurer

[PATCH] midimap: Add warning for user in case midi won't work

[Matteo Bruni]

2017-09-29 21:17 GMT+02:00 Fabian Maurer :
> Thanks, you mean I should check whether the whole string equals "midi through
> port" or what it is?

Oh, if you're intentionally just checking for the prefix then it's
fine as it is.

[PATCH] midimap: Add warning for user in case midi won't work

[Fabian Maurer]

> Oh, if you're intentionally just checking for the prefix then it's
> fine as it is.

I wasn't sure how certain the name of the port is, so I just checked the 
prefix. I initially wanted to use StrStrI, but this would mean linking against 
shlwapi, and I didn't want to add that dependency.

Regards,
Fabian Maurer