FGPUIMenuBar: fix bug when several menu entries have the same label

Before this commit, if several menu entries (possibly in different
menus) had the same label after translation and latin1-ization by
FGLocale::utf8toLatin1(), selecting one of them used to fire the
bindings associated to all such entries. This is because the bindings
used to be stored in an std::map whose keys were the
translated-and-latin1-ized labels.

This commit fixes the problem in the following way:
  - the std::map (_bindings) is turned into an std::forward_list;
  - each element of this std::forward_list references the bindings
    assigned to a menu entry;
  - in order to allow FGPUIMenuBar::fireItem() to find the bindings
    assigned to the menu entry that triggered it,
    FGPUIMenuBar::make_menu() calls puMenuBar::add_submenu() with an
    additional argument ("user data") that attaches to the puObject for
    each menu entry a pointer to the relevant element of _bindings.

Note: given how the menubar is created, an std::vector wouldn't be
appropriate for _bindings, because we need the pointers to its elements
to be stable when new elements are added to _bindings.

Reported by Wayne Bragg: https://sourceforge.net/p/flightgear/mailman/message/37682605/

src/GUI/FGPUIMenuBar.cxx

@@ -17,9 +17,10 @@
 #include "FGPUIMenuBar.hxx"
 #include "FlightGear_pu.h"
 
-using std::vector;
+using std::map;
 using std::string;
-using std::map;
+using std::unique_ptr;
+using std::vector;
 ////////////////////////////////////////////////////////////////////////
 // FIXME!!
 //
@@ -159,12 +160,13 @@ void
 FGPUIMenuBar::fireItem (puObject * item)
 {
     const char * name = item->getLegend();
-    vector<SGBinding *> &bindings = _bindings[name];
-    int nBindings = bindings.size();
+    const auto& bindings =
+        *static_cast< vector<unique_ptr<SGBinding>>* >(item->getUserData());
     flightgear::addSentryBreadcrumb("fire menu item:" + string{name}, "info");
 
-    for (int i = 0; i < nBindings; i++)
-        bindings[i]->fire();
+    for (const auto& binding: bindings) {
+        binding->fire();
+    }
 }
 
 void
@@ -185,11 +187,12 @@ FGPUIMenuBar::make_menu (SGPropertyNode * node)
 
     char ** items = make_char_array(array_size);
     puCallback * callbacks = make_callback_array(array_size);
+    const vector<unique_ptr<SGBinding>> ** userdata =
+        make_userdata_array(array_size);
 
     for (unsigned int i = 0, j = item_nodes.size() - 1;
          i < item_nodes.size();
          i++, j--) {
-
                                 // Set up the PUI entries for this item
         string label = getLocalizedLabel(item_nodes[i]);
         FGLocale::utf8toLatin1(label);
@@ -202,8 +205,14 @@ FGPUIMenuBar::make_menu (SGPropertyNode * node)
             label.append(key);
             label.append(">");
         }
+
         items[j] = strdup(label.c_str());
         callbacks[j] = menu_callback;
+        // Add an element (vector) that will contain all bindings assigned
+        // to the menu entry.
+        _bindings.emplace_front();
+        auto& bindingsVec = _bindings.front();
+        userdata[j] = &bindingsVec;
 
                                 // Load all the bindings for this item
         vector<SGPropertyNode_ptr> bindings = item_nodes[i]->getChildren("binding");
@@ -217,11 +226,12 @@ FGPUIMenuBar::make_menu (SGPropertyNode * node)
 
             binding = dest->getChild("binding", m, true);
             copyProperties(bindings[k], binding);
-            _bindings[items[j]].push_back(new SGBinding(binding, globals->get_props()));
+            bindingsVec.push_back(
+                std::make_unique<SGBinding>(binding, globals->get_props()));
         }
     }
 
-    _menuBar->add_submenu(name, items, callbacks);
+    _menuBar->add_submenu(name, items, callbacks, (void **) userdata);
 }
 
 void
@@ -263,9 +273,10 @@ FGPUIMenuBar::make_menubar(SGPropertyNode * props)
     destroy_menubar();
     _menuBar = new puMenuBar;
 
-    vector<SGPropertyNode_ptr> menu_nodes = props->getChildren("menu");
-    for (unsigned int i = 0; i < menu_nodes.size(); i++)
-        make_menu(menu_nodes[i]);
+    const vector<SGPropertyNode_ptr> menu_nodes = props->getChildren("menu");
+    for (const auto& menuNode: menu_nodes) {
+        make_menu(menuNode);
+    }
 
     _menuBar->close();
     make_object_map(props);
@@ -307,19 +318,15 @@ FGPUIMenuBar::destroy_menubar ()
     for (i = 0; i < _callback_arrays.size(); i++)
         delete[] _callback_arrays[i];
 
-                                // Delete all those bindings
-    SG_LOG(SG_GENERAL, SG_BULK, "Deleting bindings");
-    map<string,vector<SGBinding *> >::iterator it;
-    for (it = _bindings.begin(); it != _bindings.end(); it++) {
-        SG_LOG(SG_GENERAL, SG_BULK, "Deleting bindings for " << it->first);
-        for ( i = 0; i < it->second.size(); i++ )
-            delete it->second[i];
-    }
+    SG_LOG(SG_GENERAL, SG_BULK, "Deleting user data arrays");
+    for (i = 0; i < _userdata_arrays.size(); i++)
+        delete[] _userdata_arrays[i];
 
     _menuBar = NULL;
     _bindings.clear();
     _char_arrays.clear();
     _callback_arrays.clear();
+    _userdata_arrays.clear();
     SG_LOG(SG_GENERAL, SG_BULK, "Done.");
 }
 
@@ -435,4 +442,15 @@ FGPUIMenuBar::make_callback_array (int size)
     return list;
 }
 
+const vector<unique_ptr<SGBinding>> **
+FGPUIMenuBar::make_userdata_array (int size)
+{
+    auto list = new const vector<unique_ptr<SGBinding>>*[size+1];
+    for (int i = 0; i <= size; i++) {
+        list[i] = nullptr;
+    }
+    _userdata_arrays.push_back(list);
+    return list;
+}
+
 // end of menubar.cxx

src/GUI/FGPUIMenuBar.hxx

@@ -9,7 +9,10 @@
 
 #include <GUI/menubar.hxx>
 
+#include <forward_list>
 #include <map>
+#include <memory>
+#include <string>
 #include <vector>
 
 // forward decls, avoid pulling in PLIB headers here
@@ -27,9 +30,6 @@ typedef void (*puCallback)(class puObject *) ;
  * properties are not part of the main FlightGear property tree, but
  * are read from a separate file ($FG_ROOT/gui/menubar.xml).
  *
- * WARNING: because PUI provides no easy way to attach user data to a
- * menu item, all menu item strings must be unique; otherwise, this
- * class will always use the first binding with any given name.
  */
 class FGPUIMenuBar : public FGMenuBar
 {
@@ -125,8 +125,10 @@ private:
     // The top-level menubar itself.
     puMenuBar * _menuBar;
 
-    // A map of bindings for the menubar.
-    std::map<std::string,std::vector<SGBinding *> > _bindings;
+    // Each element contains the list of bindings for a particular menu entry.
+    // Not an std::vector because we want the addresses of previous elements
+    // to remain valid when we add new ones.
+    std::forward_list< std::vector<std::unique_ptr<SGBinding>> > _bindings;
 
     // These are hoops that we have to jump through because PUI doesn't
     // do memory management for lists.  We have to allocate the arrays,
@@ -134,8 +136,12 @@ private:
     // freed.
     char ** make_char_array (int size);
     puCallback * make_callback_array (int size);
+    // The return value points to an array where each element is a pointer to a
+    // vector that gives the list of bindings assigned to a given menu entry.
+    const vector<std::unique_ptr<SGBinding>> ** make_userdata_array (int size);
     std::vector<char **> _char_arrays;
     std::vector<puCallback *> _callback_arrays;
+    std::vector<const vector<std::unique_ptr<SGBinding>> **> _userdata_arrays;
 
     // A map for {menu node path}->puObject translation.
     std::map<std::string, puObject *> _objects;