From c602170faa2da56f23a5be5d68b99ff210a94d59 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Sat, 27 Oct 2018 15:27:10 +0200 Subject: [PATCH] modern-gtk2: Delay construction of Gtk::TreeModel* objects (!17) C++ initialises static member variables before main() is called. Therefore the static members of: struct Slots { static Gtk::TreeModelColumn text; static Gtk::TreeModelColumn sensitive; private: static Gtk::TreeModel::ColumnRecord record_; }; are constructed before Gtk::Main() is called in main(). However the Gtkmm documentation specifically says that they must be constructed afterwards [1]. Resolve this by using the Construct On First Use Idiom [2] to delay initialisation until the slots are first used. Normally this idiom uses static local objects, however it is being applied to class static objects here because the objects are accessed in many methods. The downside of this approach is that the objects are never destructed, which memory analysers like Valgrind could see as a memory leak, but that is actually deliberate. That leak can be removed once we can use C++11 and std::unique_ptr. [1] gtkmm: Gtk::TreeModelColumnRecord Class Reference https://developer.gnome.org/gtkmm/2.24/classGtk_1_1TreeModelColumnRecord.html#details "Neither TreeModel::ColumnRecord nor the TreeModelColumns contain any real data - they merely describe what C++ type is stored in which column of a TreeModel, and save you from having to repeat that type information in several places. Thus TreeModel::ColumnRecord can be made a singleton (as long as you make sure it's instantiated after Gtk::Main), even when creating multiple models from it. " [2] C++ FAQ / How do I prevent the "static initialization order problem"? https://isocpp.org/wiki/faq/ctors#static-init-order-on-first-use Closes !17 - Gtk2 modernisation --- include/OptionComboBox.h | 17 +++++++++------ src/OptionComboBox.cc | 46 +++++++++++++++++----------------------- 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/include/OptionComboBox.h b/include/OptionComboBox.h index c7b6d031..f3fffdde 100644 --- a/include/OptionComboBox.h +++ b/include/OptionComboBox.h @@ -144,15 +144,20 @@ public: Item_Collection items(); Item_Collection_Const items() const; - struct Slots + class Slots + : public Gtk::TreeModelColumnRecord { - static Gtk::TreeModelColumn text; - static Gtk::TreeModelColumn sensitive; + public: + Gtk::TreeModelColumn m_text; + Gtk::TreeModelColumn m_sensitive; - private: - static Gtk::TreeModel::ColumnRecord record_; - friend class OptionStore; + Slots() + { + add(m_text); + add(m_sensitive); + } }; + static Slots *m_slots; protected: explicit OptionStore(); diff --git a/src/OptionComboBox.cc b/src/OptionComboBox.cc index 8e8f70ed..b4787127 100644 --- a/src/OptionComboBox.cc +++ b/src/OptionComboBox.cc @@ -27,10 +27,7 @@ namespace GParted // NOTE: As stated in Gtkmm3 documentation, slots can be shared for all model instances. // See: Class Reference for Gtk::TreeModelColumn and Gtk::TreeModelColumnRecord. // https://developer.gnome.org/gtkmm/3.22/classGtk_1_1TreeModelColumnRecord.html#details -Gtk::TreeModelColumn OptionStore::Slots::text; -Gtk::TreeModelColumn OptionStore::Slots::sensitive; - -Gtk::TreeModel::ColumnRecord OptionStore::Slots::record_; +OptionStore::Slots *OptionStore::m_slots = NULL; OptionStore_Item::OptionStore_Item(const Glib::RefPtr& ref_model, @@ -54,36 +51,36 @@ void OptionStore_Item::set(const Glib::ustring& text, bool sensitive) { Gtk::TreeModel::iterator iter = *this; - (*iter)[OptionStore::Slots::text] = text; - (*iter)[OptionStore::Slots::sensitive] = sensitive; + (*iter)[OptionStore::m_slots->m_text] = text; + (*iter)[OptionStore::m_slots->m_sensitive] = sensitive; } void OptionStore_Item::set_text(const Glib::ustring& text) { Gtk::TreeModel::iterator iter = *this; - (*iter)[OptionStore::Slots::text] = text; + (*iter)[OptionStore::m_slots->m_text] = text; } void OptionStore_Item::set_sensitive(bool sensitive) { Gtk::TreeModel::iterator iter = *this; - (*iter)[OptionStore::Slots::sensitive] = sensitive; + (*iter)[OptionStore::m_slots->m_sensitive] = sensitive; } Glib::ustring OptionStore_Item::text() const { Gtk::TreeModel::const_iterator iter = *this; - return (*iter)[OptionStore::Slots::text]; + return (*iter)[OptionStore::m_slots->m_text]; } bool OptionStore_Item::sensitive() const { Gtk::TreeModel::const_iterator iter = *this; - return (*iter)[OptionStore::Slots::sensitive]; + return (*iter)[OptionStore::m_slots->m_sensitive]; } @@ -110,8 +107,8 @@ void OptionStore_Item_Collection::push_front(const Glib::ustring& text, bool sensitive) { Gtk::TreeModel::iterator iter = m_ref_model->prepend(); - (*iter)[OptionStore::Slots::text] = text; - (*iter)[OptionStore::Slots::sensitive] = sensitive; + (*iter)[OptionStore::m_slots->m_text] = text; + (*iter)[OptionStore::m_slots->m_sensitive] = sensitive; } @@ -119,8 +116,8 @@ void OptionStore_Item_Collection::push_back(const Glib::ustring& text, bool sensitive) { Gtk::TreeModel::iterator iter = m_ref_model->append(); - (*iter)[OptionStore::Slots::text] = text; - (*iter)[OptionStore::Slots::sensitive] = sensitive; + (*iter)[OptionStore::m_slots->m_text] = text; + (*iter)[OptionStore::m_slots->m_sensitive] = sensitive; } @@ -130,8 +127,8 @@ void OptionStore_Item_Collection::insert(const OptionStore_Item& item, { Gtk::TreeModel::iterator previous_iter = item.to_iterator_(); Gtk::TreeModel::iterator iter = m_ref_model->insert(previous_iter); - (*iter)[OptionStore::Slots::text] = text; - (*iter)[OptionStore::Slots::sensitive] = sensitive; + (*iter)[OptionStore::m_slots->m_text] = text; + (*iter)[OptionStore::m_slots->m_sensitive] = sensitive; } @@ -141,8 +138,8 @@ void OptionStore_Item_Collection::insert(unsigned position, { Gtk::TreeModel::iterator previous_iter = m_ref_model->children()[position]; Gtk::TreeModel::iterator iter = m_ref_model->insert(previous_iter); - (*iter)[OptionStore::Slots::text] = text; - (*iter)[OptionStore::Slots::sensitive] = sensitive; + (*iter)[OptionStore::m_slots->m_text] = text; + (*iter)[OptionStore::m_slots->m_sensitive] = sensitive; } @@ -269,13 +266,10 @@ OptionStore_Item_Const OptionStore_Item_Collection::operator[](unsigned position OptionStore::OptionStore() : Glib::ObjectBase("GParted_OptionStore") { - if (!Slots::record_.size()) - { - Slots::record_.add(Slots::text); - Slots::record_.add(Slots::sensitive); - } + if (!m_slots) + m_slots = new Slots(); - set_column_types(Slots::record_); + set_column_types(*m_slots); } @@ -323,8 +317,8 @@ void OptionComboBox::pack_cell_renderers() Gtk::CellRendererText *cell = manage(new Gtk::CellRendererText()); pack_start(*cell); - add_attribute(*cell, "text", OptionStore::Slots::text); - add_attribute(*cell, "sensitive", OptionStore::Slots::sensitive); + add_attribute(*cell, "text", OptionStore::m_slots->m_text); + add_attribute(*cell, "sensitive", OptionStore::m_slots->m_sensitive); }