Creating an array of struct inside another struct and accessing the members of the struct inside the-CodePudding

I'm tring to create struct (defined with typedef) array inside another struct (also defined with typedef) and access the members of the struct inside the array, but I'm kind of stuck. I really appreciate if you could guide me to the right direction.

My code is on a part of a bigger code, so I will try to explain my situation with an example code. If its not understood, I can always share my actual code.

I have two structs in a header file;

#define MAX_COMP_COUNT 64

typedef struct Module Module;
typedef struct Component Component;

struct Module
{
    Component** _CompArr[MAX_COMP_COUNT];
    uint8_t _CompCount;

};

struct Component
{
    uint8_t _page, _id;
};

I wanted to add the Component structs inside the array in the Module struct (Dynamic size is not needed at the moment.)

So I wrote this function in an .c file;

uint8_t AddComp(Module* mod, Component* _comp, uint8_t __page, uint8_t __id)
{
    //Pass the corresponding data from component to component struct
    _comp->_id = __id;
    _comp->_page = __page;

    //Add the component struct to the list on the Module Struct
    mod->_CompArr[mod->_CompCount] = &_comp;
    mod->_CompCount  ;

    //Return OK
    return 0;
}

However I'm trying to access to the id member of the struct through the array but I kind of failed. I don't know if I misused something or the parts above are defined wrong. It would be so helpful if you could correct me. Thanks in advance.

uint8_t Module_Update(Module *mod)
{
    for(uint8_t i = 0; i < mod->_CompCount; i  )
    {
        //This part is not working
        if(*(mod->_CompArr[i])->_id == 0xFF)
            //Do stuff
    }

    return 0;
}

Edit: I'm getting this error;

error: '*mod->_CompArr[(int)i]' is a pointer; did you mean to use '->'?

CodePudding user response：

Right now, you have loads of indirection that does not appear to be necessary:

struct Module
{
    Component **_CompArr[MAX_COMP_COUNT];
    uint8_t _CompCount;

};

There is no reason for the second-level pointer in Component **_CompArr. You can remove one or two levels of indirection:

With single pointers:

struct Module
{
    Component* _CompArr[MAX_COMP_COUNT];
    uint8_t _CompCount;
};

You would use the structure in your loop as

if(mod->_CompArr[i]->_id == 0xFF) ...

When you assign it, taking an extra refrence is no longer necessary:

 mod->_CompArr[mod->_CompCount] = _comp;

The second option is to do away with indirection entirely. This may not be as desirable, since it involves copying data, but it would look like this:
```
struct Module
{
    Component _CompArr[MAX_COMP_COUNT];
    uint8_t _CompCount;
};
```
Usage:
```
if(mod->_CompArr[i]._id == 0xFF) ...
```
And assignment:
```
 mod->_CompArr[mod->_CompCount] = *_comp;
```

The other alternative is of course to just fix your compiler error. Due to precedence rules, *(mod->_CompArr[i])->_id is equivalent to *((mod->_CompArr[i])->_id) rather than (*(mod->_CompArr[i]))->_id. To get the latter, you have to explicitly include the parentheses as shown.

CodePudding user response：

Drop the multiple layers of indirection. If you are just looking to have an array of fixed size, then use an array of fixed size.
Don't name identifiers starting with an underscore since such names are reserved for the compiler.

Simplified, we get this:

#include <stdint.h>

#define MAX_COMP_COUNT 64

typedef struct 
{
  uint8_t page;
  uint8_t id;
} Component;

typedef struct 
{
  Component CompArr [MAX_COMP_COUNT];
  uint8_t CompCount;
} Module;

Then modify your functions accordingly. It's a mighty weird API to pass a Component as pointer, then fill in the values of that Component struct based on other parameters - don't create weird function interfaces like that! Instead assume that the Component is already filled in by the caller instead:

void AddComp (Module* mod, Component* comp)
{
  //Add the component struct to the list on the Module Struct
  mod->CompArr[mod->CompCount] = comp;
  mod->CompCount  ;
}

Alternatively if dynamic allocation is an option:

void AddComp (Module* mod, uint8_t page, uint8_t id)
{
  Component* comp = malloc(sizeof *comp);
  comp->page = page;
  comp->id   = id;

  mod->CompArr[mod->CompCount] = comp;
  mod->CompCount  ;
}