0

I've created a menu structure in an embedded system as follows:

struct DisplayMenu
{
    short int MenuTitle;                                        // Menu title text offset - the title used in the calling menu
    char NoOfSubmenuItems;                                      // number of submenuItems in this menu ? static!
    char PositionInLastMenu;                                    // position in previous menu - helps restore previous menu screen
    struct DisplayMenu *PreviousMenu;                           // pointer to previous menu - need to be const?
    struct DisplayMenu *NextMenu[MAX_NO_OF_SUBMENU_ITEMS];      // array of pointers to submenus structs - Null if no submenu...function call instead
    void (*MenuFunctionsArray[MAX_NO_OF_SUBMENU_ITEMS])(void);  // array of function pointers - called if no submenu. Check if null
    const short int *SubMenuText;                               // pointer array of text offsets for submenu text - no set length variable length array allowed at end of struct in GCC at least
};

Which is instantiated thus for early prototyping:

const short int MainMenuTextStrings[]= { tSTATUS, tSETUP, tStartOfTextString, tANALYSER, tUNITS, tEndOfTextString, tUNITS, tALARM, tSCREEN, tREPORTS, tSERVICE, tStartOfTextString, tMANUAL, tAIR, tZERO, tEndOfTextString, tStartOfTextString, tMANUAL, tEndOfTextString, NULL };     // needs a new text entry

// main menu struct
struct DisplayMenu MainMenu =
{
    tMENU,                                                                                              // Menu title
    0,                                              // number of submenus
    0,                                                                                                  // no previous menu 
    NULL,                                                                                               // no previous menu to point to
    { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL },                                     // to be set up later
    { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL },                                     // function pointers
    MainMenuTextStrings
};

with a view to initialising function/menu pointers and the like later

It's compiling ok with GCC but when the code is running (there is a non-dynamic menu system up and running already) the IDE (MPLABX) is struggling to view the contents MainMenu and when I try and run the function DisplayMenuItems (as below) I can't step through it

Can anyone spot anything wrong with the way I've defined the struct and instantiated it?

I plan to use the following function to display items using the struct:

void DisplayMenuItems(struct DisplayMenu* Menu)
{

    while(*(Menu->SubMenuText) != NULL)
    {
        if(*(Menu->SubMenuText) == tStartOfTextString)       //if a few text strings need to be concatenated on one line
        {
            while(*(Menu->SubMenuText++) != tEndOfTextString)  //keep printing until you find the EndofTextString ident
                WriteStringToDisplay((char*) &pText[Config.ucLanguage][*(Menu->SubMenuText)], SmallFont);
        }
        else /*if(*(Menu->SubMenuText) != NULL) */
        {
            WriteStringToDisplay((char*) &pText[Config.ucLanguage][*(Menu->SubMenuText)++], SmallFont);
        }
        DisplayCarriageReturn(2,SmallFont);         // try and remove this eventually? Just increment Lcd.ucLine instead
    }
}

GCC might be letting me get away with something I'm not supposed to here so any tips appreciated!

Many thanks

  • That cast and buried `*(Menu->SubMenuText)++` doesn't look healthy at all. – WhozCraig Aug 06 '14 at 11:47
  • What is the purpose of `while(*(Menu->SubMenuText) != NULL)`? `*(Menu->SubMenuText)` returns `short int` value. Comparing it to `NULL` makes no sense at all. – user694733 Aug 06 '14 at 12:15

1 Answers1

2

The problem is probably that you are modifying the pointer in the loop. That means that if the DisplayMenuItems function is called a second time, the pointer will no longer point to the MainMenuTextStrings array, but beyond it and accessing that data leads to undefined behavior.

Instead of modifying the pointer, why not simply use array indexing syntax?

Like

for (unsigned int i = 0; Menu->SubMenuText[i] != NULL; ++i)
{
    ...
}
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • 1
    +1 Not your fault, but if any of the OP's choices for the mystery values like `tSTATUS`, etc, are `0`, neither this, nor their code, can work. That NULL comparison is a bad idea (again, you didn't bring it to the party; the OP did). A separate member `SubMenuTextCount` would make this considerably cleaner imho. – WhozCraig Aug 06 '14 at 11:51
  • I'm using tSTATUS and the like as offsets into an array of text strings. So I have an array of these offsets for each menu. I have a NULL int entry at the end of this array to know when to stop. But yes will consider using SubMenuTextCount instead – mr_Alex_Nok_ Aug 06 '14 at 12:37