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

Use proper type instead of "int32" for OSAL IDs #171

Closed
jphickey opened this issue Jan 13, 2022 · 0 comments · Fixed by #175
Closed

Use proper type instead of "int32" for OSAL IDs #171

jphickey opened this issue Jan 13, 2022 · 0 comments · Fixed by #175
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

Describe the bug
In some functions, e.g. CF_WriteQueueDataToFile, CF_WriteHistoryQueueDataToFile, the OSAL file descriptor is being passed as an int32. This is incorrect, but allowed by the backward-compatibility features in OSAL. In a future version, this will become an error.

To Reproduce
Build OSAL with type-safe ID. Implicit conversion of ID to/from an integer will fail to compile.

Expected behavior
Use osal_id_t type instead, which is the correct type for an OSAL ID.

Code snips

int32 CF_WriteQueueDataToFile(int32 fd, CF_Channel_t *c, CF_QueueIdx_t q);

int32 CF_WriteHistoryQueueDataToFile(int32 fd, CF_Channel_t *c, CF_Direction_t dir);

System observed on:
Ubuntu 21.10

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Jan 13, 2022
jphickey added a commit to jphickey/CF that referenced this issue Jan 13, 2022
Use the correct typedef for OSAL ID.  This also necessiates using
the correct conversion macro where use as an integer is intended.
jphickey added a commit to jphickey/CF that referenced this issue Jan 13, 2022
Use the correct typedef for OSAL ID.  This also necessiates using
the correct conversion macro where use as an integer is intended.
astrogeco added a commit that referenced this issue Jan 18, 2022
Fix #171, use osal_id_t for OSAL ID, not int32
@skliper skliper added this to the Draco milestone Mar 24, 2022
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