Simplify LogOverflowCounter increment logic

[skliper]

Is your feature request related to a problem? Please describe.
The CFE_EVS_Global.EVS_LogPtr->LogOverflowCounter gets incremented in two locations and is done based on the LogFullFlag:

cFE/modules/evs/fsw/src/cfe_evs_log.c

Lines 52 to 64 in c8b5e00

	if ((CFE_EVS_Global.EVS_LogPtr->LogFullFlag == true) &&
	(CFE_EVS_Global.EVS_LogPtr->LogMode == CFE_EVS_LogMode_DISCARD))
	{
	/* If log is full and in discard mode, just count the event */
	CFE_EVS_Global.EVS_LogPtr->LogOverflowCounter++;
	}
	else
	{
	if (CFE_EVS_Global.EVS_LogPtr->LogFullFlag == true)
	{
	/* If log is full and in wrap mode, count it and store it */
	CFE_EVS_Global.EVS_LogPtr->LogOverflowCounter++;
	}

Describe the solution you'd like
Pull up higher and change the if/else statement to:
if(CFE_EVS_Global.EVS_LogPtr->LogMode != CFE_EVS_LogMode_DISCARD)

Describe alternatives you've considered
None

Additional context
Code review

Requester Info
Jacob Hageman - NASA/GSFC

[thnkslprpt]

@skliper - Unless I'm misreading the logic here, most of the code in the else block, from the memcpy down, is also executed in the following path/circumstances:
LogFullFlag == false
LogMode == CFE_EVS_LogMode_DISCARD

Is this intentional?

Executing that block with the memcpy etc. based on if(CFE_EVS_Global.EVS_LogPtr->LogMode != CFE_EVS_LogMode_DISCARD) would therefore not be logically equivalent - it would miss that path/case mentioned above.

[skliper]

No behavior change intended. I think the code review comment was suggesting something like the following pseudocode:

if (LogFull)
  Increment log full flag

if (!LogFull || DISCARD)
  The rest of the logic to add the event to the log and count it

[thnkslprpt]

Yes but again that is the issue I see - I'm wondering whether it's intentional or not.
From what I can tell, the logically equivalent pseudocode (to the current code) would be:

if (LogFull)
  Increment log full flag

if (!LogFull || !DISCARD)
  The rest of the logic to add the event to the log and count it

Note the ! before DISCARD.
The above code changes, when implemented, passes all tests.

That's why I'm wondering whether the rest of the code (to add the event data to the log etc.) is actually supposed to occur in the case of LogFull && !DISCARD - which is what happens now.

Hopefully this makes sense.

[skliper]

@thnkslprpt - Good catch! Definitely !DISCARD. Want to add to the log either if the log isn't full or if not set to discard mode. The Logfull && !DISCARD would only add to the log if it's full (which wouldn't ever happen if you can't add to the log when it's not full).

[thnkslprpt]

Cool no worries - hopefully the PR makes it a little clearer for future maintenance 😅