Add struct name for typedef struct in cFE

[mlouielu]

Is your feature request related to a problem? Please describe.

All of the struct used in cFE are declared with typedef struct {} TYPEDEF_NAME, which make the struct anonymous in debug info, and make it harder to access from the debugging tools such as `pahole:

$ pahole -E -M -C "CFE_EVS_ShortEventTlm_t" build/exe/cpu1/core-cpu1
typedef struct  CFE_EVS_ShortEventTlm_t;

Expect to be like:

$ pahole -M -C "CFE_EVS_LongEventTlm_t" build/exe/cpu1/core-cpu1
struct CFE_EVS_LongEventTlm_t {
	uint8                      TlmHeader[12];        /*     0    12 */
	CFE_EVS_LongEventTlm_Payload_t Payload;          /*    12   156 */

	/* size: 168, cachelines: 3, members: 2 */
	/* last cacheline: 40 bytes */
};

and

$ pahole -E -M -C "CFE_EVS_LongEventTlm_t" build/exe/cpu1/core-cpu1
struct CFE_EVS_LongEventTlm_t {
	/* typedef uint8 -> uint8_t */ unsigned char              TlmHeader[12];         /*     0    12 */
	/* typedef CFE_EVS_LongEventTlm_Payload_t */ struct CFE_EVS_LongEventTlm_Payload_t {
		/* typedef CFE_EVS_PacketID_t */ struct CFE_EVS_PacketID_t {
			char       AppName[20];                                          /*    12    20 */
			/* typedef uint16 -> uint16_t */ short unsigned int EventID;     /*    32     2 */
			/* typedef uint16 -> uint16_t */ short unsigned int EventType;   /*    34     2 */
			/* typedef uint32 -> uint32_t */ unsigned int SpacecraftID;      /*    36     4 */
			/* typedef uint32 -> uint32_t */ unsigned int ProcessorID;       /*    40     4 */
		} PacketID; /*    12    32 */
		char               Message[122];                                         /*    44   122 */
		/* --- cacheline 2 boundary (128 bytes) was 38 bytes ago --- */
		/* typedef uint8 -> uint8_t */ unsigned char      Spare1;                /*   166     1 */
		/* typedef uint8 -> uint8_t */ unsigned char      Spare2;                /*   167     1 */
	} Payload; /*    12   156 */

	/* size: 168, cachelines: 3, members: 2 */
	/* last cacheline: 40 bytes */
};

Describe the solution you'd like

Add struct name to all the typedef struct in cFE, for example:

From:

typedef struct {
   uint8                    TlmHeader[CFE_SB_TLM_HDR_SIZE];
   CFE_EVS_ShortEventTlm_Payload_t Payload;

} CFE_EVS_ShortEventTlm_t;

Changed to:

typedef struct CFE_EVS_ShortEventTlm_t {
   uint8                    TlmHeader[CFE_SB_TLM_HDR_SIZE];
   CFE_EVS_ShortEventTlm_Payload_t Payload;

} CFE_EVS_ShortEventTlm_t;

Describe alternatives you've considered

Additional context

Requester Info

@mlouielu

[CDKnightNASA]

surprised any tools in use today don't understand the "typedef struct {} foo_t;" form. But not difficult to add. I suggest a ticket also be filed against github.com/nasa/osal and github.com/nasa/psp ?

[jphickey]

In general I agree that it can be beneficial to also give the struct a name in addition to the typedef. But the struct name should not end in _t - that's for typedefs. So just struct CFE_EVS_ShortEventTlm should do it, then you don't get a name overlap/ambiguity between them.

[CDKnightNASA]

In general I agree that it can be beneficial to also give the struct a name in addition to the typedef. But the struct name should not end in _t - that's for typedefs. So just struct CFE_EVS_ShortEventTlm should do it, then you don't get a name overlap/ambiguity between them.

I vote for CFE_EVS_ShortEventTlm_s for the struct. I often write code like Foo_t Foo; Foo.bar = "baz"; ...

[CDKnightNASA]

I'd like to take this on, but I'd like concurrence on naming. I am ok with no suffix at all, but would prefer:

#include <stdio.h>

typedef struct StructExample_s
{
    int Field;
} StructExample_t;

typedef union UnionExample_u
{
    int Field;
} UnionExample_t;

typedef enum EnumExample_e
{
    A, B, C
} EnumExample_t;

int main(int argc, char **argv)
{
    StructExample_t StructExample = { 1 };
    UnionExample_t UnionExample = { 2 };
    EnumExample_t EnumExample = C;
}

[jphickey]

I still vote for no suffix - it is easier to read and I don't see what value the suffix adds here. Whenever these names are referenced it must already be preceded by a struct, union, or enum keyword, respectively - so to also add a suffix of the same is just redundant.

Typedefs OTOH appear "bare" in code (no keyword) so an _t suffix is perhaps more justified - or at least traditional - so developers are used to seeing it. But nobody uses these suffixes on struct/union/enum names as far as I know.

[msuder]

Late to the party, but just an interesting and subtle note on something I recently ran across related to this item and differences between C and C++ (I know cFE/cFS is C). The following is valid C code, but not valid C++ code:

struct foo{};
typedef struct bar{} foo;
typedef struct baz{} baz;
foo f1;
struct foo f2;
baz f3;
struct baz f4;

The reason being (if I understand correctly... I didn't totally go back to the C and C++ standards) that in C, the named structs and the typedefs are kept separately... whereas in C++, a struct is basically a class and C++ removed the need to say "class foo" or "struct foo" when declaring variables/parameters, etc., so the typedefs and the structs/classes all end up in the same name space.

Now I admit, the example code is very evil and I never want to see anyone do anything like it... but it does point out that you need to be careful about names of typedefs and names of structs/classes and their potential for collision.

In the end, I might make a case that a tool that does not understand anonymous structs with typedef names is not doing things correctly (although I don't know if pahole can necessarily tell if binary code came from a C or C++ compiler)... food for thought.

[CDKnightNASA]

nom nom food for thought...

I think we're ok, all of our typedefs are of the form: typedef struct CFE_MOD_Type { ... } CFE_MOD_Type_t; /* note the _t */

So little/no collision risk and if there ever is a collision, that's on us.