diff --git a/Makefile b/Makefile index 2d2c80e..35ae36d 100644 --- a/Makefile +++ b/Makefile @@ -612,3 +612,7 @@ test: test-midi-file .PHONY: test-configuration-startup test-configuration-startup: gui xvfb-run -a -s "-screen 0 1440x960x24" $(PYTHON) tests/integration/test_configuration_startup.py ./$(BUILD_DIR)/xtc + +.PHONY: test-gui-midi-labels +test-gui-midi-labels: $(BUILD_DIR)/test_gui + XTC_TEST_MIDI_LABELS_ONLY=1 xvfb-run -a -s "-screen 0 1440x960x24" ./$(BUILD_DIR)/test_gui diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 77ab658..4650f1a 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -2264,6 +2264,15 @@ Provide device dropdowns and refresh in Preferences, persistent default input/ou ### Implementation status Implemented: a MIDI preferences tab provides editable source/destination dropdowns, device refresh, input filtering, output channel defaults and optional thru. All choices use the strict shared configuration schema and ~/.xtc.conf. Play/Record restores selected routes; missing devices reject startup without losing choices. Settings roundtrip/invalid-value, GUI discovery/refresh/persistence/missing-device checks and the full GUI suite passed. MIDI help explains host bridge exposure, channel numbering and existing note/CC defaults. + +Follow-up fix: MIDI dropdown labels now use dialog-owned backing strings because +FLTK copies menu items but borrows their labels. Menus are cleared before refresh +or string destruction. The regression test reproduces the previous heap +use-after-free under AddressSanitizer and verifies both lists, literal names with +slashes, selection/save, repeated refresh, offline selection retention and None. +`test-gui-midi-labels`, `test-gui-midi-workflows`, full `test-gui` and the focused +ASan/UBSan/leak check pass; the GUI application was rebuilt without warnings. + --- ## Phase 103 — Export Selected Clips as Standard MIDI Files diff --git a/src/gui/preferences_dialog.hpp b/src/gui/preferences_dialog.hpp index f3b2c15..7619cf1 100644 --- a/src/gui/preferences_dialog.hpp +++ b/src/gui/preferences_dialog.hpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include @@ -17,11 +19,17 @@ class PreferencesDialog : public Fl_Double_Window { std::vector inputs_; Fl_Box *message_; std::function(bool)> devices_; + std::vector midi_names_[2]; Fl_Input_Choice *midi_input_=nullptr,*midi_output_=nullptr; void refresh_devices() { for(auto pair:{std::pair{midi_input_,true},{midi_output_,false}}){ if(!pair.first)continue; - std::string value=pair.first->value();std::vector names=devices_ ? devices_(pair.second) : std::vector{}; + std::string value=pair.first->value(); + // copy() owns the menu array but borrows its label strings. + // Release old items before replacing their backing storage. + pair.first->menubutton()->clear(); + auto &names=midi_names_[pair.second ? 0 : 1]; + names=devices_ ? devices_(pair.second) : std::vector{}; names.insert(names.begin(),"");std::vector items(names.size()+1); for(size_t i=0;imenubutton()->copy(items.data());pair.first->value(value.c_str()); @@ -89,6 +97,11 @@ class PreferencesDialog : public Fl_Double_Window { callback([](Fl_Widget *,void *c){static_cast(c)->hide();},this); end(); xtc_ui::style(this); } + ~PreferencesDialog() override { + // Destroy borrowed-label menus before the member strings are released. + if(midi_input_)midi_input_->menubutton()->clear(); + if(midi_output_)midi_output_->menubutton()->clear(); + } void run() { show(); while(shown()) Fl::wait(); } }; #endif diff --git a/tests/integration/test_gui.cpp b/tests/integration/test_gui.cpp index 71f64bb..fc83d1d 100644 --- a/tests/integration/test_gui.cpp +++ b/tests/integration/test_gui.cpp @@ -399,8 +399,38 @@ int midi_workflow_tests() CHECK(xtc_settings_load(&saved,config.c_str())==XTC_ERR_FORMAT); unlink(config.c_str());rmdir(dir);puts("MIDI workflow/configuration GUI checks passed.");return failures ? 1 : 0; } +int midi_device_label_tests() +{ + xtc_settings settings{},applied{};xtc_settings_defaults(&settings);unsigned int generation=0; + auto names=[&](bool input){return std::vector{ + std::string(input ? "Controller / USB keyboard: MIDI output " : "Synth / USB interface: MIDI input ")+std::string(120,'A'+static_cast(generation)), + input ? "Keyboard:out 2" : "Synth:in 2"};}; + PreferencesDialog dialog(settings,"",[&](const xtc_settings &s){applied=s;return XTC_OK;},names); + auto *input=dynamic_cast(find(&dialog,"Default MIDI input source (next Play)")); + auto *output=dynamic_cast(find(&dialog,"Default MIDI output destination (next Play)")); + CHECK(input && output);if(!input || !output)return 1; + for(unsigned int pass=0;pass<4;++pass){ + if(pass){generation=pass;press(find(&dialog,"Refresh MIDI devices"));} + // Force reuse of temporary string allocations after enumeration returns. + std::vector churn(256,std::string(200,'z')); + for(auto pair:{std::pair{input,true},{output,false}}){ + auto expected=names(pair.second);auto *menu=pair.first->menubutton(); + CHECK(menu->size()==4 && !strcmp(menu->menu()[0].label(),"(None)")); + for(size_t i=0;imenu()[i+1].label(),expected[i].c_str())); + menu->picked(menu->menu()+1);CHECK(pair.first->value()==expected[0]); + } + press(find(&dialog,"Apply")); + CHECK(applied.midi_input_port==names(true)[0] && applied.midi_output_port==names(false)[0]); + } + auto selected=std::string(input->value());generation=5;press(find(&dialog,"Refresh MIDI devices")); + CHECK(input->value()==selected); // A disconnected device remains remembered. + for(auto *choice:{input,output}){choice->menubutton()->picked(choice->menubutton()->menu());CHECK(!*choice->value());} + press(find(&dialog,"Apply"));CHECK(!applied.midi_input_port[0] && !applied.midi_output_port[0]); + puts("MIDI device dropdown labels/selection/refresh checks passed.");return failures ? 1 : 0; +} int main(int argc, char **argv) { + if(std::getenv("XTC_TEST_MIDI_LABELS_ONLY"))return midi_device_label_tests(); if(std::getenv("XTC_TEST_MIDI_WORKFLOWS_ONLY"))return midi_workflow_tests(); if(std::getenv("XTC_TEST_CREATIVE_ONLY"))return creative_workflow_tests(); if(std::getenv("XTC_TEST_TRACK_FX_ONLY"))return track_insert_tests();