Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CF method CF_CFDP_MsgOutGet returns 0x10 when it should be NULL #28

Closed
jphickey opened this issue Nov 23, 2021 · 3 comments · Fixed by #137
Closed

CF method CF_CFDP_MsgOutGet returns 0x10 when it should be NULL #28

jphickey opened this issue Nov 23, 2021 · 3 comments · Fixed by #137
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1726] CF method CF_CFDP_MsgOutGet returns 0x10 when it should be NULL
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Fri Sep 10 14:28:59 2021

Original Description:
CF_CFDP_MsgOutGet has a path where it will return a 0x10 if the CF_AppData.engine.out.msg is NULL. There are two if blocks after the msg NULL verification that if neither come back true the assignment on line 382 of cf_cfdp.c:

ret = &((pdu_s_msg_t*)CF_AppData.engine.out.msg)->ph;

will make ret == 0x10, not NULL.
The description of the return statement shows that this is not correct:

\retstmt Pointer to a pdu_header_t within a software bus buffer on success. Otherwise NULL.

@jphickey
Copy link
Contributor Author

Imported comment by internal user on Mon Sep 27 10:06:10 2021

(link removed)

This line does not check the return result from CFE_MSG_Init and continues on as though the value put into &CF_AppData.engine.out.msg->Msg is good even if it failed. If it comes back NULL, it will return a value of 0x10 because ph is 0x10 away from the base pointer in the message type, 0x10 from 0x00.

It is unknown if this situation can happen or not, but the unit tests ran into it.

@jphickey
Copy link
Contributor Author

I noted this as well in my testing, it appears this can happen any time the "frozen" or "suspended" flags are set true. This causes it to skip the actual msg allocation, so the pointer is NULL, but it still executes this line:

ret = &((CF_PduSendMsg_t *)CF_AppData.engine.out.msg)->ph;

This means the function will return something that is not NULL but also not valid (0x10 is likely when using the default MSG implementation where telemetry header size is 16 bytes).

@jphickey
Copy link
Contributor Author

Should definitely be fixed....!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants