Fix #788, Simplified CFE_EVS_SendEvent macros

[CDKnightNASA]

Closes #788

Describe the contribution
Macros for more compact calls to CFE_EVS_SendEvent, making the type be part of the fn name.

Contributor Info - All information REQUIRED for consideration of pull request
Christopher.D.Knight@nasa.gov

[CDKnightNASA]

Should I, as part of this PR, change a bunch of cFE code to use these simplified macro calls?

[astrogeco]

Should I, as part of this PR, change a bunch of cFE code to use these simplified macro calls?

I like keeping things separate. So at the very least do it in separate commits. Another option is to provide a sample implementation or documentation with this PR. And, once the approach is "approved" by the CCB open a PR to update the cFE code.

[iamthad]

This could be done with an inline function instead of a macro if a version of CFE_EVS_SendEvent that takes va_list (as requested in #249) were implemented.

[ghost]

Suggestion for less duplicate code:

#define CFE_EVS_SendDbg(E,...) CFE_EVS_SendEvent((E), CFE_EVS_EventType_DEBUG, __VA_ARGS__)
#define CFE_EVS_SendInfo(E,...) CFE_EVS_SendEvent((E), CFE_EVS_EventType_INFORMATION, __VA_ARGS__)
#define CFE_EVS_SendErr(E,...) CFE_EVS_SendEvent((E), CFE_EVS_EventType_ERROR, __VA_ARGS__)
#define CFE_EVS_SendCrit(E,...) CFE_EVS_SendEvent((E), CFE_EVS_EventType_CRITICAL, __VA_ARGS__)

instead:

#define CFE_EVS_Send(E,T,...) CFE_EVS_SendEvent((E), CFE_EVS_EventType_##T, __VA_ARGS__)
#define CFE_EVS_SendInfo(E,...) CFE_EVS_Send(E, INFORMATION, __VA_ARGS__)

... and so on

[skliper]

Suggestion for less...

@klystron78 - copy, a helper macro for the helper macros. I'm fine with either approach.

[ghost]

Looks like double underscore bolds in the comments. There's probably a way to do a code block or something like that.

[skliper]

@Klystron single back quote (just learned also known as birk, blugle, or grave) inline code or triple

for block of code

[CDKnightNASA]

This could be done with an inline function instead of a macro if a version of CFE_EVS_SendEvent that takes va_list (as requested in #249) were implemented.

I'm confused what the benefit is of an inline (which, of course, isn't always inline) over a macro. Can you describe/prototype?

[CDKnightNASA]

Suggestion for less duplicate code:

#define CFE_EVS_SendDbg(E,...) CFE_EVS_SendEvent((E), CFE_EVS_EventType_DEBUG, __VA_ARGS__)
#define CFE_EVS_SendInfo(E,...) CFE_EVS_SendEvent((E), CFE_EVS_EventType_INFORMATION, __VA_ARGS__)
#define CFE_EVS_SendErr(E,...) CFE_EVS_SendEvent((E), CFE_EVS_EventType_ERROR, __VA_ARGS__)
#define CFE_EVS_SendCrit(E,...) CFE_EVS_SendEvent((E), CFE_EVS_EventType_CRITICAL, __VA_ARGS__)

instead:

#define CFE_EVS_Send(E,T,...) CFE_EVS_SendEvent((E), CFE_EVS_EventType_##T, __VA_ARGS__)
#define CFE_EVS_SendInfo(E,...) CFE_EVS_Send(E, INFORMATION, __VA_ARGS__)

... and so on

I like, I'll change, thanks!

[iamthad]

This could be done with an inline function instead of a macro if a version of CFE_EVS_SendEvent that takes va_list (as requested in #249) were implemented.

I'm confused what the benefit is of an inline (which, of course, isn't always inline) over a macro. Can you describe/prototype?

General advice these days seems to be to prefer inline functions over function-like macros. For example, see SEI CERT Recommendation PRE00-C. I admit, for this sort of trivial wrapper, the benefit is probably small and probably outweighed by the additional lines of code.

It would look something like the following (using a hypothetical CFE_EVS_VSendEvent):

static inline int32 CFE_EVS_SendDbg(uint16 EventID, const char *Spec, ...)
{
    va_list ap;
    va_start(ap, Spec);
    int32 res = CFE_EVS_VSendEvent(EventID, CFE_EVS_EventType_DEBUG, Spec, ap);
    va_end(ap);
    return res;
}

[jphickey]

I'm generally a fan of inline functions rather than macros because they are more type-safe, and they offer more predictable/defined behavior as to when things are evaluated and when side effects occur, such as the classic example of a macro that evaluates its arguments more than once.

Admittedly they aren't quite as useful in C as they are in C++, where you have templates. So in C we need a explicitly-defined inline function for every type variant, whereas a macro or C++ template can be evaluated at compile-time for any type.

It is true they are not always actually inlined - it's a suggestion to the compiler, but depends on compiler capabilities, settings, etc as to whether it accepts that suggestion.

But I'm not sure if any compiler can inline something that uses va_list because AFAIK it depends on having those arguments in a stack frame. If truly inline-d then there is no stack frame for it. For anything that uses variable arguments/va_list then it is probably not worth trying to inline it.

[astrogeco]

@CDKnightNASA is this ready to merge?

[CDKnightNASA]

@CDKnightNASA is this ready to merge?

yes please

[skliper]

@astrogeco - what's the status on this one?

[astrogeco]

@astrogeco - what's the status on this one?

Whoops, looks like this never made it to a CCB since it hasn't been "github reviewed". Tagging it for next week

Edit-1: Unless we did review it and I forgot to comment on it :( I'm looking through the old agendas right now
Edit-2: Looks like we had queued for CCB 2020-09-09 but didn't get approved then

[astrogeco]

@skliper or we could fast-track it

[astrogeco]

CCB 2020-11-04 APPROVED

@CDKnightNASA can you rebase/squash the commits?