fix #1055 - CFE_SB_ReceiveBuffer stub returns timeout or empty if buffer

[CDKnightNASA]

Closes #1055

This changes the return value returned by the CFE_SB_ReceiveBuffer() stub when there has not been a buffer defined.

Testing performed
Tested with SB and SBN unit tests.

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

[skliper]

How about only setting the return codes if status is != 0?

I'd expect if I force the return code (non-zero) it should always work... currently it would override any non-zero forced return code.

[CDKnightNASA]

How about only setting the return codes if status is != 0?

I'd expect if I force the return code (non-zero) it should always work... currently it would override any non-zero forced return code.

Good question, I can see if I can figure out if the default return value has been changed or deferred and not override the return value if it has...But are there cases where someone would override the return value and not provide a buffer?

[skliper]

How about only setting the return codes if status is != 0?
I'd expect if I force the return code (non-zero) it should always work... currently it would override any non-zero forced return code.

Good question, I can see if I can figure out if the default return value has been changed or deferred and not override the return value if it has...But are there cases where someone would override the return value and not provide a buffer?

All I was thinking is for the if (UT_Stub_CopyToLocal <= 0), add && status == 0 prior to modifying the return status code locally

[jphickey]

FYI - When a stub needs to "know" if a return code was explicitly set by the test case or not, easiest way is to use a "sentinel" value that differs from the default or any other known error, i.e. INT32_MAX or something like that. Use the UT_DEFAULT_IMPL_RC() macro to do this. If the status comes back as that same sentinel value, it means the test case did not set a status.

[skliper]

After letting it sink in a bit - I'm actually OK with it as is if you want. Like it was said, there probably isn't a valid use case for a >= 0 forced return value if the buffer wasn't set.

[jphickey]

My recommendation is to minimize the amount of "special sauce" in the stubs, such as special return codes, whenever possible. This special info should be in the test case setup, not in the stub. The exceptions to this are where we had existing test cases that depended on the stub behaving a certain way (such as several TIMEOUT responses, in the case of the old CFE_SB_RcvMsg). But since this is a new stub, we shouldn't have to do that hack.

In cases where there is an output data buffer required then the stub just contains an internal one (i.e. a static local variable) and that is output/returned if the test case didn't register one. It can be just zeroed out.

But at the end of the day, returning CFE_SUCCESS is fine, if the test case didn't register anything. By definition, if the test case actually cares about having a specific/non-success error code from this function, it should register that status code in its setup.

[astrogeco]

CCB 2021-01-06 MODIFY

• Avoid having "special sauce" in stubs and keep them as generic as possible and only do what the test requires

[CDKnightNASA]

20200106 - CCB rejected, suggested correcting documentation to remove reference to timeout, cdknight will re-create pull to correct comments.

[astrogeco]

@CDKnightNASA have you worked on refactoring this?

[skliper]

This is OBE, erroneous documentation was removed as part of stub refactor (update to stub/handler model).