Truncate MIDI SysEx messages after termination byte

Joerg-Cyril.Hoehle at Joerg-Cyril.Hoehle at
Tue Jan 15 04:45:32 CST 2013


Christian costa wrote:
>+            lpNewData[lpMidiHdr->dwBufferLength + len_add] = 0xF7;
Here you're using spaces only, whereas the surrounding code uses TAB8.

>I tested it with the unit test attached.
Instead of some isolated unit test, it would be preferable to
add to winmm/tests/midi.c such that midiOutLongMsg gets covered
by the tests. But I don't know whether there's some harmless
general SysEx that we can send out.  Maybe some MIDI guru could
suggest some?

The fix looks good, however while we are busy fixing lpNewData handling,
I'd like to see the same patch fix its broken memory handling:

1. In the MOD_FMSYNTH branch, no
         HeapFree(GetProcessHeap(), 0, lpNewData);
Here I'm surprised that the static analysers did not spot it.

2. Wrong pointer being freed -- is that an indirect proof by absence of
crash that this missing F0/F7 situation was never seen in practice?
1007         if (lpNewData)
1008                 HeapFree(GetProcessHeap(), 0, lpData);
Also, Wine style is to suppress the if()

3. Not checking for success
971         lpNewData = HeapAlloc(GetProcessHeap(), 0, lpMidiHdr->dwBufferLength + 2);
Either exit or skip adding F0/F7

	Jörg Höhle

More information about the wine-devel mailing list