[Scummvm-tracker] [ScummVM :: Bugs] #16644: AUDIO: Valgrind warning in MidiDriver_MT32GM (Lure of the Temptress, Riddle of Master Lu, ...)

[ScummVM :: Bugs]

#16644: AUDIO: Valgrind warning in MidiDriver_MT32GM (Lure of the Temptress, Riddle
of Master Lu, ...)
-------------------------+--------------------
Reporter:  eriktorbjorn  |      Owner:  (none)
    Type:  defect        |     Status:  new
Priority:  normal        |  Component:  Audio
 Version:                |   Keywords:
    Game:                |
-------------------------+--------------------
 I see the following Valgrind warning when starting The Riddle of Master Lu
 with MIDI music in the current ScummVM development version:

 {{{
 ==279868== Conditional jump or move depends on uninitialised value(s)
 ==279868==    at 0xC37EFAB: MidiDriver_MT32GM::controlChange(unsigned
 char, unsigned char, unsigned char, signed char,
 MidiDriver_MT32GM::MidiChannelControlData&, bool) (mt32gm.cpp:597)
 ==279868==    by 0xC380907: MidiDriver_MT32GM::applySourceVolume(unsigned
 char) (mt32gm.cpp:1146)
 ==279868==    by 0xC369920: MidiDriver_Multisource::syncSoundSettings()
 (mididrv_ms.cpp:387)
 ==279868==    by 0xC37D771: MidiDriver_MT32GM::open(MidiDriver*, bool)
 (mt32gm.cpp:192)
 ==279868==    by 0xC37D52E: MidiDriver_MT32GM::open() (mt32gm.cpp:165)
 ==279868==    by 0x9538655: M4::Sound::Midi::open() (midi.cpp:114)
 ==279868==    by 0x94FA52B: M4::Vars::game_systems_initialize(unsigned
 char) (vars.cpp:139)
 ==279868==    by 0x94FA199: M4::Vars::init() (vars.cpp:80)
 ==279868==    by 0x94F5DFA: M4::M4Engine::run() (m4.cpp:93)
 ==279868==    by 0x6CAA038: runGame(Plugin const*, OSystem&, DetectedGame
 const&, void const*) (main.cpp:324)
 ==279868==    by 0x6CAC53B: scummvm_main (main.cpp:818)
 ==279868==    by 0x6CA6E9F: main (posix-main.cpp:44)
 }}}

 If I understand the Valgrind warning correctly,
 `MidiDriver_Multisource::syncSoundSettings()` calls
 `applySourceVolume(0xFF)`.

 `MidiDriver_MT32GM::applySourceVolume()` calls `controlChange()` if source
 is `0xFF` or if `_controlData[i]->source == source`. So I guess `0xFF
 means "everything".

 But `_controlData[i]->source` is `-1` so the `source` parameter to
 `MidiDriver_MT32GM::controlChange()` is not a valid index into the
 `_sources` array. So that feels like a crash waiting to happen.

 At this point, the M4 engine has not even tried to allocate any channels.
 In fact, I can't see that it ever does. I thought I could look at the Lure
 of the Temptress engine for inspiration, since it ''does'' allocate
 channels. But it seems that it has the same problem.
-- 
Ticket URL: <https://bugs.scummvm.org/ticket/16644>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM