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

Incorrect parameter type in Timebase sync callback #664

Closed
jphickey opened this issue Dec 1, 2020 · 0 comments · Fixed by #666 or #680
Closed

Incorrect parameter type in Timebase sync callback #664

jphickey opened this issue Dec 1, 2020 · 0 comments · Fixed by #666 or #680
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

jphickey commented Dec 1, 2020

Is your feature request related to a problem? Please describe.
Timebase sync callback prototype is currently defined as:

typedef uint32 (*OS_TimerSync_t)(osal_index_t timer_id); /**< @brief Timer sync */

But indices (table position) is an internal OSAL value that shouldn't be used externally from OSAL. More importantly, it is easy to alias, and cannot be differentiated if an object is deleted and then created again.

Describe the solution you'd like
Use the full ID value, not the index. So the prototype would be:

typedef uint32 (*OS_TimerSync_t)(osal_id_t timer_id); /**< @brief Timer sync */

Describe alternatives you've considered
Leave as is.

Additional context
I was going to roll this into a larger change, but figured this technically qualifies as an "API change" so writing it as a separate issue for specific awareness. However, nothing outside of OSAL itself (and the included tests) actually implements a sync callback. PSPs could, but none currently do, so this really shouldn't have any current impact to users. But if users do start using this option, better to have the full ID value than just the index.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Dec 1, 2020
jphickey added a commit to jphickey/osal that referenced this issue Dec 2, 2020
ID is preferable to an array index because direct table access
should not be done outside OSAL itself.
jphickey added a commit to jphickey/osal that referenced this issue Dec 2, 2020
ID is preferable to an array index because direct table access
should not be done outside OSAL itself.
jphickey added a commit to jphickey/osal that referenced this issue Dec 2, 2020
ID is preferable to an array index because direct table access
should not be done outside OSAL itself.
jphickey added a commit to jphickey/osal that referenced this issue Dec 2, 2020
ID is preferable to an array index because direct table access
should not be done outside OSAL itself.
jphickey added a commit to jphickey/osal that referenced this issue Dec 4, 2020
ID is preferable to an array index because direct table access
should not be done outside OSAL itself.
@astrogeco astrogeco added this to the 6.0.0 milestone Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants