0

I'm attempting to populate a "submenu" in my dialog box from an array of strings as shown in this answer.

My attempt looks like the following:

#define ID_APP0 14000
#define ID_APP1 14001
#define ID_APP2 14002
#define ID_APP3 14003
#define ID_APP4 14004
#define ID_APP5 14005
#define ID_APP6 14006
#define ID_APP7 14007

void SoftwareDlg::DynamicAppMenu()
{
    CMenu MyMainMenu;
    VERIFY(MyMainMenu.LoadMenu(IDR_MENU1));
    
    CMenu* SomeMenu = MyMainMenu.GetSubMenu(0);
    if (SomeMenu)
    {
        for (auto i = 0; i < 1; i++)
        {
            SomeMenu->AppendMenu(MF_STRING, 14000+i, Client::m_vszAppArr[i]);
        }
    }
}

...but I'm getting an exception from the assert below immediately after(during?) the AppendMenu() function.

_AFXWIN_INLINE BOOL CMenu::AppendMenu(UINT nFlags, UINT_PTR nIDNewItem, LPCTSTR lpszNewItem)
    { ASSERT(::IsMenu(m_hMenu)); return ::AppendMenu(m_hMenu, nFlags, nIDNewItem, lpszNewItem); }

I no longer know how to continue debugging this issue because the LoadMenu() function appears to work correctly, and none of the variables are populated where the exception takes place.

Am I possibly just calling this in the wrong place? It's happening(conditionally) inside of a scheduled member function of the dialog box... does it need to happen somewhere like OnDraw(), OnPaint(), or something?

Edit1: Value of SettingsMenu->m_hMenu m_hMenu

Edit2: IDR_MENU1 resource definition:

IDR_MENU1 MENU
BEGIN
    POPUP "Tools"
    BEGIN
        MENUITEM "New Test                  F11", ID_TOOLS_NEWTEST
        MENUITEM "Export Data",                 ID_TOOLS_EXPORTDATA
        MENUITEM "Upload Data",                 ID_TOOLS_UPLOADDATA
        MENUITEM "Test Control",                ID_TOOLS_TESTCONTROL
    END
    POPUP "Settings"
    BEGIN
        POPUP "USB_PORT"
        BEGIN
            MENUITEM "Serial Ports",                ID_PORT_SERIALPORTS, INACTIVE
            MENUITEM "COM1",                        ID_PORT_COM1
            MENUITEM "COM2",                        ID_PORT_COM2
            MENUITEM "COM3",                        ID_PORT_COM3
            MENUITEM "COM4",                        ID_PORT_COM4
            MENUITEM "COM5",                        ID_PORT_COM5
            MENUITEM "COM6",                        ID_PORT_COM6
            MENUITEM "COM7",                        ID_PORT_COM7
            MENUITEM "COM8",                        ID_PORT_COM8
        END
        MENUITEM "Debug Mode",                  ID_SETTINGS_DEBUGMODE
        MENUITEM "Display in OSD",       ID_SETTINGS_DISPLAYINOSD
        MENUITEM "Test Upload Mode",            ID_SETTINGS_TESTUPLOADMODE
        MENUITEM "Preferences",                 ID_SETTINGS_PREFERENCES
        POPUP "Target Window"
        BEGIN
            MENUITEM "Placeholder",                 ID_TARGETWINDOW_PLACEHOLDER
        END
    END
END
Skewjo
  • 379
  • 3
  • 12
  • 1
    From [the documentation](https://learn.microsoft.com/en-us/cpp/mfc/reference/cmenu-class?view=msvc-160#getsubmenu): *The CMenu pointer returned should not be stored.* So, maybe just use `MyMainMenu.GetSubMenu(0)->AppendMenu(...)` in each loop? – Adrian Mole Jan 04 '21 at 20:40
  • 1
    What's the value of `m_hMenu` of the `CMenu*` returned from `MyMainMenu.GetSubMenu(0)`? – IInspectable Jan 04 '21 at 20:42
  • @AdrianMole, thanks for the reply. I was really hoping that was it, but sadly that didn't fix it. @IInspectable, I added a picture illustrating the value of ```SettingsMenu->m_hMenu```. Appears to be "unused", but I'm not sure how it is supposed to be "used". – Skewjo Jan 04 '21 at 20:50
  • 1
    @Skewjo Ignore the `unused` fields, those are not actually used for anything meaningful. You are compiling your code with [STRICT Type Checking](https://docs.microsoft.com/en-us/windows/win32/winprog/strict-type-checking) enabled, so `HMENU` is a typedef of a `struct HMENU__` used only for type safely. The contents of that struct are irrelevant. What is important is that `SettingsMenu` and `SettingsMenu->m_hMenu` is both not `NULL` pointers. And BTW, your screenshot shows that you are accessing `SettingsMenu->m_hMenu` before verifying whether `SettingsMenu` is `NULL` or not. Don't do that. – Remy Lebeau Jan 04 '21 at 21:00
  • 2
    @Skewjo can you please [edit] your question to include the `IDR_MENU1` resource definition? According to [the answer](https://stackoverflow.com/a/1160099/2934222) you linked to: "*Use your resource editor **to add a submenu containing one placeholder item**. You can then programatically grab a reference to this submenu, add items to it and delete the placeholder item*" - do you actually have such a placeholder in your menu resource? – Remy Lebeau Jan 04 '21 at 21:05
  • @RemyLebeau, thanks for the reply. Do I need to turn off STRICT Type Checking in order to check that ```SettingsMenu->m_hMenu``` is not null? Also, you can see the "placeholder" I was attempting use in the resource definition under "Settings"->"Target Window"->"Placeholder". I stopped using them because I thought part of my problem was attempting to access the menu using an incorrect "position". – Skewjo Jan 04 '21 at 21:37
  • @Skewjo "*Do I need to turn off STRICT Type Checking in order to check that SettingsMenu->m_hMenu is not null?*" - No, of course not. `m_hMenu` is still a pointer, whether it is a `void*` pointer (non-STRICT) or a `struct HMENU__*` pointer (STRICT). "*Also, you can see the "placeholder" I was attempting use in the resource definition under "Settings"->"Target Window"->"Placeholder".*" - that is not the menu item your code is accessing. You are trying to append to the "Settings" menu item, not the "Target Window" menu item. In fact, you are not even accessing the "Settings" popup menu at all – Remy Lebeau Jan 04 '21 at 21:40
  • @RemyLebeau, yes sorry, I must have not stated what I meant very clearly. I stopped attempting to use the placeholder, because I was getting the same assertion exception, but thought it may have been caused by an incorrect "position" argument to ```GetSubMenu()```. I can edit the question to show my original code, but I believe the problem is the same either way. – Skewjo Jan 04 '21 at 21:45
  • 1
    Re: `Am I possibly just calling this in the wrong place? ` - possibly. Where are you calling it from? What is `scheduled member function of the dialog box.`? Is it a menu attached to some window, or a pop-up? In first case, you should use `GetMenu()`, otherwise your `MyMainMenu` will just get destroyed when that function returns. – Vlad Feinstein Jan 05 '21 at 04:23
  • Thank you for the hint @VladFeinstein. Using ```CMenu MyMainMenu``` to create a new menu was not the correct course of action. Moving this declaration out of htis function and into the class allowed me to get past the assertion exception, but then the menu wasn't actually being created... Because I already an existing menu, I needed to use a pointer to that existing menu and go from there. It's working beautifully now. Thanks for the help. – Skewjo Jan 05 '21 at 14:52

1 Answers1

0

As hinted to by @Vlad Feinstein's comment, CMenu MyMainMenu; should not have been declared inside the function... That fix allowed me to get past the assertion, but the new menu items were still not appearing. I began catching and logging the return values from the creation and deletion functions which lead me to realize that because I already had an existing menu, I needed to use a pointer to that menu(as opposed to creating my own that I then left un-used).

My final function ended up looking like this:

void SoftwareDlg::DynamicAppMenu(){
    CMenu* MainMenu = GetMenu();
    CMenu* SettingsMenu = MainMenu->GetSubMenu(1);
    CMenu* TargetAppMenu = SettingsMenu->GetSubMenu(5);
    
    if (TargetAppMenu)
    {
        BOOL appended = false;
        BOOL deleted = false;

        for (auto i = 0; i < Client::m_vszAppArr.size(); i++)
        {
            appended = TargetAppMenu->AppendMenu(MF_STRING, 14000+i, Client::m_vszAppArr[i].c_str());
        }
        deleted = TargetAppMenu->DeleteMenu(ID_TARGETWINDOW_PLACEHOLDER, MF_BYCOMMAND);
        
        OutputDebugString(("String appended: " + std::to_string(appended)).c_str());
        OutputDebugString(("Placeholder deleted: " + std::to_string(deleted)).c_str());
    }
    
}
Skewjo
  • 379
  • 3
  • 12