0

I'm working on an api to create menu structures for my stm32 project (stm32f0). I've created a struct that represents an item in the menu, and navigation up and down levels is tracked by establishing a relationship of pointers to parent and child structs in the menu. There are 2 different types, folders, and targets depending on whether a menu item is a setting/parameter (target), or just a folder.

Here is my function to create a new folder item. The parentItem is passed to the function which inheretly creates the link for the new menu item, however because a folder (parent) may contain multiple child items, I need to dynamically allocate memory to hold the pointer for the new child in the parent. I'm having difficulty understanding the correct pointer syntax to achieve this as. I'm getting the error "incompatible types when assigning to type 'struct MenuItems' from type 'MenuItem * {aka struct MenuItems *}'"

Here is my .h file with the typedefs:

typedef enum
{
    Folder,
    Target
} ItemType;

typedef struct MenuItems
{
    struct MenuItems *parent;   // pointer to parent level item (-1 level)
    struct MenuItems *child;    // pointer to child level items (+1 level) (FOLDER ONLY)
    uint8_t numChildren;        // number of child items held by this item (FOLDER ONLY)
    ItemType type;              // whether item is a folder or target
    uint8_t index;              // assigned index of this item
    uint8_t visible;            // flag to set whether the item is visible to the user
    void (*func)();             // pointer to the function when target is chosen
    char name[ITEM_NAME_LENGTH];// displayed string for the menu item

} MenuItem;

And this is my function in the .c to create a new folder item:

MenuItem *menu_addFolder(MenuItem *parentItem, char name[])
{
    // newIndex is used to assign the index to the new item
    // Due to 0 indexing, this is used verbatim
    // e.g. if numChildren = 1, the index assigned will be 1 as there are now 2 items
    // By assigning the index before incrementing newChildren, 0 indexing is handled
    uint8_t newIndex = parentItem->numChildren;
    parentItem->numChildren++;
    // create item and allocate memory
    MenuItem *newItem = malloc(sizeof(MenuItem));
    newItem->child = malloc(sizeof(MenuItem));
    newItem->parent = malloc(sizeof(MenuItem));
    // check for memory allocation errors
    if(newItem == NULL || newItem->child == NULL || newItem->parent == NULL)
    {
        return NULL;
    }

    newItem->numChildren = 0;
    newItem->type = Folder;
    newItem->index = newIndex;
    newItem->func = NULL;
    strncpy(newItem->name, name, ITEM_NAME_LENGTH);
    // establish the child-parent links
    newItem->parent = parentItem;

    // When a new folder is created, memory for a single child is allocated
    // So for the first child assignment per parent, no new memory allocation is required
    // However, for consecutive child assignments, new memory allocation is required
    // If this is the first child assigned to the parent item
    if(parentItem->numChildren == 1)
    {    
        parentItem->child = newItem;
    }

    // If there are existing items assigned to the parent item
    else if(parentItem->numChildren > 1)
    {
    parentItem->child = realloc(parentItem->child, sizeof(MenuItem) * parentItem->numChildren);
        if(parentItem->child == NULL)
        {
            return NULL;
        }
        parentItem->child[newIndex] = newItem;
    }

    return newItem;
    }

Thanks in advance!

5
  • 1
    You should always indicate which line of code causes the compiler error. Commented Jan 31, 2020 at 23:52
  • 5
    Code does newItem->parent = malloc(sizeof(MenuItem)); and then a few lines later: newItem->parent = parentItem;, throwing away the previous allocation. Certainly the first assignment was not needed. Commented Jan 31, 2020 at 23:53
  • 1
    While it is possible to point out obvious issues in a function, it is impossible to tell whether that is the root of the problems you are seeing without A Minimal, Complete, and Verifiable Example (MCVE). With a MCVE the workings of your code can be compiled and validated. Commented Feb 1, 2020 at 0:14
  • Note that void (*func)(); doesn't specify the argument list required by the function — the argument list can be any fixed list of arguments (not variable arguments like printf(); that requires a proper prototype with ellipsis , ... at the end of the signature. It is best to avoid such non-prototype pointers to functions, though it can sometimes be fraught to do so. (This is wholly tangential to the problem you currently face; it may become relevant later, though.) Commented Feb 1, 2020 at 0:33
  • @samspencer Using malloc in the uC design is a bad habit and should be avoided. Many embedded coding standards explicit ban the dynamic allocation. Yuor code shows that you just started with the uCs and have habits form the big machines with almost infinitive resources. Commented Feb 1, 2020 at 10:52

2 Answers 2

1

because a folder (parent) may contain multiple child items, I need to dynamically allocate memory to hold the pointer for the new child in the parent.

Well, no, you don't necessarily need dynamic allocation for that. You could instead choose a fixed upper bound on the number of children each folder may have, and put an array of that dimension in struct MenuItems. Since you're talking about storing only pointers to the children, that would look something like this:

typedef struct MenuItems
{
    struct MenuItems *parent;
    struct MenuItems *child[MAX_CHILDREN];
    uint8_t numChildren;
    // ...

But the dynamic allocation approach does give you more flexibility, and probably also more efficient memory usage, at the cost of code complexity. Most people would take that trad-off. But now compare the above with the corresponding part of your actual declaration:

typedef struct MenuItems
{
    struct MenuItems *parent;   // pointer to parent level item (-1 level)
    struct MenuItems *child;    // pointer to child level items (+1 level) (FOLDER ONLY)
    uint8_t numChildren;        // number of child items held by this item (FOLDER ONLY)

Compare carefully the types of the two different declarations of MenuItem.child. They are in no way interchangeable.

I'm getting the error "incompatible types when assigning to type 'struct MenuItems' from type 'MenuItem * {aka struct MenuItems *}'"

I'm sure you are. That would arise from this statement:

        parentItem->child[newIndex] = newItem;

, given that the type of newItem is struct MenuItems *.

That statement would be correct for the array alternative I presented, because there the type of parentItem->child[newIndex] is also struct MenuItems *. But that's why I asked you to compare. In your case, the type of parentItem->child[newIndex] is struct MenuItems (that is, not a pointer). That would be appropriate for storing the child menu items themselves, instead of pointers to them. And that would be a viable approach too.

Supposing that you want to stick with your stated strategy, however, and have each folder item store pointers to its children, you need to modify struct MenuItems appropriately:

typedef struct MenuItems
{
    struct MenuItems *parent;
    struct MenuItems **child;
    uint8_t numChildren;
    // ...

You will then also want to reduce the size of your allocations for the child member, because again, you say you want to store pointers there, not the children themselves.

Sign up to request clarification or add additional context in comments.

3 Comments

Hi John, that's a really helpful answer, thanks for taking the time. So if I understand it correctly, my main issue was using "struct MenuItems child}"instead of "struct MenuItems **child". So to correctly allocate the memory only needed to store the pointer, my malloc/realloc size should simple be "sizeof(struct MenuItems)", is that correct?
@SamSpencer, Reading what I think you actually typed through the formatting as which SO interpreted it, yes, that is correct.
Oh yes, sorry. I meant to say sizeof(struct MenuItems*). Thanks again for your help!
1

The compilation error appears to be on this line of code:

parentItem->child[newIndex] = newItem;

You're using child as a pointer to an extensible array of menu items (note: items not pointers to item), extending it by calling realloc(). The problem is that, while you've created new space at the end of parentItem->child for the new item, you've tried to assign the newItem pointer to the child array, when you would need to memcpy() the item itself into the new space you've created, and then free the recently-allocated newItem. For example:

memcpy(&parentItem->child[newIndex], newItem, sizeof(*newItem));
free(newItem);

A better alternative would be to change child from array of struct MenuItems to effectively be an array of pointer to struct MenuItems, then you could simply assign the newly-allocated item.

Comments

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.