Calculate crc can't return error code

[zanzaben]

Is your feature request related to a problem? Please describe.
Calculate CRC returns a uint so any error code you use gets overloaded.

Describe the solution you'd like
be able to handle errors

Additional context
Was found while working on #547

Requester Info
Alex Campbell GSFC

[skliper]

Issue - a buggy app could call this API with a NULL pointer and cause a segfault. Better to have an API that returns status, and sets the CRC as an output parameter.

[jphickey]

IMO This falls into the category of where the checks themselves are adding complexity that otherwise wouldn't exist. A "buggy app" is unlikely to call this function with NULL, it would more likely call it with some random pointer which is not NULL but not valid either. So a "safety check" will not catch it.

But since nobody expects or would anticipate something like a CRC could ever fail (it is, after all, a mathematical algorithm applied to a bunch of numbers) the extra complexity of adding a int32 return code and actually expecting users to check that code is just a lot of extra work for stuff that should never actually happen.

If coding standards actually require that all functions accept a NULL pointer without segfault, then I'd propose just returning the CRC init value, e.g.:

if (DataPtr == NULL || DataLength == 0)
{
    return InputCRC;
}

That way the function just becomes a no-op if passed with a NULL pointer. I still say that this is only likely to "defer" a failure and make it more complicated to debug - but if coding standards require the function to accept NULL this is an easy way to do it and does not change API or make it unnecessarily complicated.

[skliper]

I'm fine w/ the suggestion above (make it a no-op, return InputCRC). No API change is good for @ejtimmon...

[zanzaben]

A similar issue happens with CFE_SB_GetChecksum.

cFE/fsw/cfe-core/src/sb/cfe_sb_util.c

Lines 231 to 252 in c7363c8

	/*
	* Function: CFE_SB_GetChecksum - See API and header file for details
	*/
	uint16 CFE_SB_GetChecksum(CFE_MSG_Message_t *MsgPtr)
	{
	CFE_MSG_Type_t type = CFE_MSG_Type_Invalid;
	bool hassechdr = false;
	CFE_MSG_GetHasSecondaryHeader(MsgPtr, &hassechdr);
	CFE_MSG_GetType(MsgPtr, &type);
	/* if msg type is telemetry or there is no secondary hdr... */
	if((type == CFE_MSG_Type_Tlm)||(!hassechdr))
	{
	return 0;
	}/* end if */
	/* Byte access for now to avoid error if secondary doesn't contain checksum */
	return MsgPtr->Byte[sizeof(CCSDS_SpacePacket_t) + 1];
	}/* end CFE_SB_GetChecksum */

currently if you send a null pointer, CFE_MSG_GetHasSecondaryHeader will catch it leaving hassechdr false and returning 0.

[skliper]

CFE_SB_GetChecksum is deprecated. Don't need to worry about it.