Search code examples
cstructenumsmicrochip

C struct compilation fails


I'm working on a driver for the Microchip Harmony Framework. It looks like a Linux driver. I've got a struct (NRF24L01_MainStateInfo) that stores all the state needed by the driver, it's just a "collection" made of enums. It's been like 2 days that I'm struggling to this:

../../../../framework/driver/nrf24l01/src/states/initialization_state/../../../drv_nrf24l01.h:51:2: error: unknown type name 'NRF24L01_MainStateInfo''

The struct that has a member of that type (and where the error points) is the following one:

#ifndef __DRV_NRF24L01_H__
#define __DRV_NRF24L01_H__

// Framework include
//...
// Project specific include
#include "src/memory_map.h"
#include "src/nrf_definitions.h"
#include "src/states/drv_nrf24l01_main_state.h" // NRF24L01_MainStateInfo defined here
#include "src/bus/drv_nrf24l01_bus.h"
//...
typedef struct _nrf24l01_driver_info {
    // Driver in use? (already configured)
    bool inUse;
    // Driver's mutex
    OSAL_MUTEX_HANDLE_TYPE drvMutex;
    // Driver configuration
    NRF24L01_ConfigData config;
    // Client count. Useless since there is a mapping 1:1 between driver and client
    uint8_t clientCnt;
    // Driver system status (returned by status)
    SYS_STATUS status;
    // FSM state
    NRF24L01_MainStateInfo state; // <-- This member generate the error

    // Bus information
    NRF24L01_BusVTable vTable;
    void *busInfo;

} NRF24L01_DriverInfo;

//...

#endif

The structure NRF24L01_MainStateInfo is declared in src/states/drv_nrf24l01_main_state.h as follow:

#ifndef __DRV__NRF24L01_MAIN_STATE_H__
#define __DRV__NRF24L01_MAIN_STATE_H__

//#include "../../drv_nrf24l01.h"
#include "initialization_state/drv_nrf24l01_init_state.h"

struct _nrf24l01_driver_info;

/*
  Main driver state. These are the state that the developer will see.
*/
typedef enum {
  NRF24L01_MAIN_STATE_UNINITIALIZED = 0,
  NRF24L01_MAIN_STATE_INITIALIZATION,
  NRF24L01_MAIN_STATE_RUNNING,
  NRF24L01_MAIN_STATE_CLOSING,
  NRF24L01_MAIN_STATE_CLOSED
} NRF24L01_MAIN_STATE;

typedef struct _nrf24l01_mainstate_info {
  NRF24L01_MAIN_STATE mainState;
  NRF24L01_INIT_STATE initState;
} NRF24L01_MainStateInfo;

int32_t DRV_nRF24L01_MainStateTask(struct _nrf24l01_driver_info *pDrv);

#endif /* end of include guard: __DRV__NRF24L01_MAIN_STATE_H__ */

Now I can't figure why this error came up.

The directory tree is the following:

nrf24l01 .
│   drv_nrf24l01.h
│   LICENSE
│   README.md
│
├───config
│       .gitignore
│       drv_nrf.hconfig
│       drv_nrf24l01.hconfig
│       drv_nrf24l01_idx.ftl
│
├───src
│   │   drv_nrf24l01.c
│   │   memory_map.h
│   │   nrf_definitions.h
│   │
│   ├───bus
│   │   │   drv_nrf24l01_bus.h
│   │   │
│   │   └───spi
│   │           drv_nrf24l01_spi.c
│   │           drv_nrf24l01_spi.h
│   │
│   ├───internal
│   │       drv_nrf_internal.c
│   │       drv_nrf_internal.h
│   │
│   └───states
│       │   drv_nrf24l01_main_state.c
│       │   drv_nrf24l01_main_state.h
│       │
│       ├───closing_state
│       ├───initialization_state
│       │       drv_nrf24l01_init_state.c
│       │       drv_nrf24l01_init_state.h
│       │
│       └───running_state
└───templates
        system_config.h.ftl
        system_definitions.h.INC.ftl
        system_definitions.h.OBJ.ftl
        system_init.c.DRV.ftl
        system_init.c.INIT.ftl
        system_interrupt.c.ftl
        system_tasks.c.ftl

Maybe I'm missing something?

The compiler is the xc32-gcc and the uC is a PIC32MX110F016B.


Solution

  • You have a circular header dependency, which is bad design and almost always leads to failures. The problem is complicated by an overly-complicated naming strategy which makes the code really difficult to read.

    Here's the basic problem, using significantly simplified names:

    File driver.h

    #ifndef DRIVER_H
    #define DRIVER_H
    
    #include "state.h"
    
    /* See answer text for an explanation of this declaration style. */
    typedef struct Driver Driver;
    struct Driver {
      // ...
      State state;
      // ...
    };
    
    #endif
    

    File state.h

    #ifndef STATE_H
    #define STATE_H
    
    // Needed because the Driver type is used in a prototype
    #include "driver.h"
    
    typedef struct State State;
    struct State {
      // ...
    };
    
    // Attempt to fix circular dependency with a redundant declaration.
    typedef struct Driver Driver;
    int32_t stateTask(Driver* driver);
    #endif
    

    So these two headers include each other. The header guards will keep them from being included twice, but they won't guarantee that the declarations are read in the correct order. What will happen depends on which order you #include the two headers:

    • If you first #include "driver.h", it will set its header guard and immediately #include "state.h". It hasn't been included before, so the header guard isn't set and the compiler starts processing it. It immediately hits #include driver.h", but that header guard is now set, even though the header hasn't really yet been processed, so circular inclusion is avoided. Eventually reaches the prototype which references the type Driver, which hasn't been defined yet.

      I assume you already hit this problem because of the redundant struct declarations in state.h. After inserting these declarations, you could have removed the #include "driver.h" but perhaps you had some other need for it.

    • On the other hand, if you first #include "state.h", the compiler sees that its header guard hasn't been set, sets the header guard, and proceeds with the header. It immediately sees #include "driver.h"; that header guard has not yet been set, so it sets that header guard and proceeds with that header. Now when it hits the #include "state.h" in the driver.h header, it does nothing because the header guard has now been set.

      Unfortunately, that #include is really necessary. Compilation fails when an attempt is made to define a member of type State, which has not yet been defined. In this case, you're actually including an entire State object, not just using a pointer, so you can't get away with just forward declaring the struct tab.

    In short, things will work OK with one header include order, but fail with a different one. Unfortunately, it's very hard to predict the order in which headers will be included in a complex project, because header includes are not always visible. They may occur inside other headers which are being included. This can lead to headers being included in the "wrong" order.

    In general, it is not a good idea to write headers which must be #included in a particular order. It almost always ends in this kind of problem.

    When you have several inter-related types all being used by the same small component, you are better off putting all of them into a single header. You will still need to sort the order out correctly, but at least they are all in one place. In that single header file, it may be necessary to forward-declare structures in order to allow pointers to them from other structures. Putting the type definitions ahead of the prototypes reduces the need to forward-declare for prototype references.

    If you have a lot of types, you could put all of their declarations in a single internal project/types.h header. Then you can arrange the prototypes in whatever complicated file organisation you fancy. This is a pretty common paradigm. For external prototype headers -- that is, headers which declare functions intended to be globally visible -- you might be able to reduce clutter by just forward declaring the structs being used by the prototypes. Assuming the prototype only uses a pointer to the struct, which is certainly the most common, there is no need to make the definition of the struct visible.

    Warning: Opinionated style recommendations follow. If you don't like that sort of thing, you can stop reading here.

    Once you have your headers sorted out, assuming they are only being used internally, you can simplify things for yourself and your readers by dropping unnecessary prefixes from internal structure and enum names. Typedefs, structure and union tags, and enums do not have linkage; they cannot leak into another separately compiled translation unit. So there is no need to make them globally unique if they are not intended for global use.

    Regardless of what you might see in other people's code, there is absolutely no need to make typedef names different from struct tags, even if you someday intend to compile with C++. In C, the names are in two completely separate namespaces; a struct tag is only recognised as such when preceded with the token struct. So

    typedef struct MyStructure MyStructure;
    

    is absolutely valid. In fact, it is valid even if struct MyStructure has not yet been fleshed out, making it convenient for structure types which include pointers to the same type.

    I tend to use the style shown in the code snippets above, always putting the typedef before the struct definition. I find that more readable than having the typedef name way down at the end of the struct definition, even if in my style the names are always the same. Also, the forward typedefs can simply be copied to headers which need them. C does not complain if you typedef the same name to the same type more than once, so this is totally safe.