0

I'm developing a simple but flexible menu system for an embedded system in C. Due to limitations of the platform , I want to avoid dynamic allocation and want everything to be defined statically.

I have an Menu type and a MenuItem type defined as structs, with a Menu containing multiple MenuItems. Each menu item will have a Label, ItemOptionType and then a pointer to an array of Options.

That part that's not working for me is creating a reference to the Option array. I get a warning saying

warning: incompatible pointer types initializing 'char *' with an expression of type 'char *(*)[3]' [-Wincompatible-pointer-types]
MenuItem menu_item1 = {"Day", OPTION_LIST, 0,0,0, 4, &dayOptionList};

Any suggestions on the right approach would be much appreciated.

#define MAX_MENU_ITEMS  16


typedef enum {OPTION_LIST, OPTION_NUMERIC, OPTION_BINARY} ItemOptionType;

typedef struct MenuItem {
    char* label;
    ItemOptionType optionType; 
    unsigned char min;
    unsigned char max;
    unsigned char value;
    unsigned char optionCount;
    char* optionList[];
} MenuItem;

typedef struct Menu {
    unsigned char count;
    MenuItem* items[MAX_MENU_ITEMS];
} Menu;


unsigned int MenuAddItem(Menu* menu, MenuItem* item) {
    if (menu->count < MAX_MENU_ITEMS) {
        menu->items[menu->count] = item;
        menu->count++;
        return 1;
    }
    
    return 0;
}

char* dayOptionList[] = {"Monday", "Tuesday", "Wednesday" "Thursday", "Friday", "Saturday", "Sunday"};

Menu mainMenu;
MenuItem menu_item1 = {"Day", OPTION_LIST, 0,0,0, 4, &dayOptionList};
MenuItem menu_item2 = {"Age", OPTION_NUMERIC, 0, 120, 25, 0, NULL};


int main(int argc, char *argv[]) {

    MenuAddItem(&mainMenu, &menu_item1);
    MenuAddItem(&mainMenu, &menu_item2);
    while(1);
}
4
  • 3
    char* optionList[]; is a flexible array member. You can't have those without dynamic allocation. What you probably want is something like char **. And then initialize optionList using plain dayOptionList (without the pointer-toi operator *). Commented Oct 4, 2021 at 10:49
  • By the way, the error (or warning) you get doesn't match the code you show, assuming you get it at the initialization of menu_item1. Please copy-paste the full and complete build output from the code you actually show. Commented Oct 4, 2021 at 10:50
  • Well, you can have flexible array members without heap allocation but then the memory allocation has to be handled manually by setting up memory sections at fixed addresses etc. Usually that's a very cumbersome solution, I did it in one project but wasn't pleased with the results - too much extra complexity. Commented Oct 4, 2021 at 10:56
  • I've corrected the sample with the full compiler output. Sorry about that. Commented Oct 4, 2021 at 11:08

2 Answers 2

2
typedef struct mmMenuItem {
   ...
   char* optionList[];
} MenuItem;

In the above declaration the last line is a very special 'flexible array' declaration. It allows having structs of arbitrary sizes in 'c'. Also, whehen you declare any array with '[]', it is not a real pointer, all its members must be present in the struct.

In your case you just need a pointer to a statically allocated array of strings, which can be expressed as

typedef struct mmMenuItem {
   ...
   char** optionList;
} MenuItem;

This declaration statically allocates array of 7 elements. Compiler does it for you:

char* dayOptionList[] = {"Monday", "Tuesday", "Wednesday" "Thursday", "Friday", "Saturday", "Sunday"};

Array names in 'c' represetn their addresses as well. So, when you pass a pointer to the array in params, you do not need to use &.

MenuItem menu_item1 = {"Day", OPTION_LIST, 0,0,0, 4, dayOptionList};
                                                   ^^^^^
Sign up to request clarification or add additional context in comments.

Comments

1

The error comes from trying to assign a char* to an array pointer, which are not compatible types.


First of all, char* dayOptionList[] is wrong on any system, since string literals are read-only. But it is especially wrong in embedded systems, because you want this table to be allocated in flash, not RAM. Correct code is:

const char* dayOptionList[] = { ...

Now if you have a list of strings like this:

const char* dayOptionList[] = {"Monday", "Tuesday", "Wednesday" "Thursday", "Friday", "Saturday", "Sunday"};

Then the sensible member type to use in the struct is a const char**, to point at the first member in the above array. char* optionList[]; declares a flexible array member, which is briefly put, the wrong solution to a different problem.


However, we need to know the size of the array somehow too. This can be done in two ways:

  • Either include a NULL sentinel value at the end of the above array. This is a memory over speed optimization, we can't have trivial direct access, but have to iterate through the container item by item.

  • Or include a separate size member in the struct, which is probably the most sensible thing to do in most use-cases. If we know the size, we can do a direct access to an item in the array without risking out-of-bounds access.


Misc code review unrelated to the question:

  • You should consider your struct layout with alignment in mind, so there will be no needless, wasted padding bytes in the middle. The compiler is not allowed to re-order the struct, so this is the programmer's responsibility.

  • You should stop using the "primitive data types" int, unsigned char etc since those are non-deterministic and therefore non-portable. Professional embedded systems always use the type system of stdint.h.

  • int main(int argc, char *argv[]) is the wrong format for microcontroller embedded systems. Those use impl.defined void main (void) instead. If you are using gcc, compile with -ffreestanding which means embedded system target.

2 Comments

Thanks for the feedback. As for the misc code review. I'm developing the library on a desktop machine before moving it over to the MCU to ease debugging, hence the use of int main(int argc, char *argv[]). But great other points. Will make those changes.
@Dibly You can use -ffreestanding on gcc for PC too, if I remember correctly.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.