Should message buffers returned from CFE_SB_ReceiveBuffer be checked for NULL?

[thnkslprpt]

Checklist

• I reviewed the Contributing Guide.
• I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
With regard to this comment:

CF/fsw/src/cf_app.c

Lines 280 to 284 in 831fb16

	/*
	* note that CFE_SB_ReceiveBuffer() guarantees that a CFE_SUCCESS status is accompanied by
	* a valid (non-NULL) output message pointer. However the unit test can force this condition.
	*/
	if (status == CFE_SUCCESS && msg != NULL)

Based on the logic here:
https://github.com/nasa/cFE/blob/37f1d28df44aa7a858555b92f0c20310a6803b9f/modules/sb/fsw/src/cfe_sb_api.c#L1890-L1944
and a bit further down here:
https://github.com/nasa/cFE/blob/37f1d28df44aa7a858555b92f0c20310a6803b9f/modules/sb/fsw/src/cfe_sb_api.c#L1989-L1993

it does indeed seem (as per the comment in CF) that CFE_SB_ReceiveBuffer guarantees that the msg/buf pointer will either:

1. definitely be valid (not NULL), if CFE_SB_ReceiveBuffer returns CFE_SUCCESS, or
2. definitely be NULL, if CFE_SB_ReceiveBuffer returns anything other than CFE_SUCCESS

Therefore, is this a situation where the code is being designed around the tests, rather than the other way around?

Can this check of the buffer != NULL be removed?
Note: the cFE core and most of the apps do not check the buffer for NULL when it's returned from CFE_SB_ReceiveBuffer with CFE_SUCCESS

This would also be an issue for achieving full MC/DC coverage no?

Expected behavior
Remove unnecessary conditions and unreachable paths.

Reporter Info
Avi Weiss   @thnkslprpt