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.

Sign up for GitHub

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

Jump to bottom

Update OS_U32ValueWrapper_t name and consider using uintptr_t #869

Closed
skliper opened this issue Mar 10, 2021 · 1 comment · Fixed by #901 or #917
Closed

Update OS_U32ValueWrapper_t name and consider using uintptr_t #869

skliper opened this issue Mar 10, 2021 · 1 comment · Fixed by #901 or #917
Assignees
@jphickey
Labels
CFS-38 enhancement
Milestone
Caelum

Comments

@skliper
Copy link
Contributor

skliper commented Mar 10, 2021

Is your feature request related to a problem? Please describe.
Confusion in name where U32 is used for a union that'll hold a void *. Also recommendation to consider using uintptr_t

Describe the solution you'd like
Consider rename/uintptr_t use.

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC, OSAL code review
@skliper skliper added this to the 6.0.0 milestone Mar 10, 2021
@skliper
Copy link
Contributor Author

skliper commented Mar 10, 2021

If name is appropriate (it is "wrapping" a u32, even though the union is likely 64) maybe add a comment to that affect so the intent is clear.

jphickey added a commit to jphickey/osal that referenced this issue Mar 12, 2021
@jphickey
Fix nasa#869, rename OS_U32ValueWrapper_t
0b94a7a 
The "U32" designator in the name had become confusing because all
the members had over time migrated to a non-uint32/dedicated type.

The point is to pass a value through a void* so a name change better
indicates that purpose.
@jphickey jphickey mentioned this issue Mar 12, 2021
Merged
jphickey added a commit to jphickey/osal that referenced this issue Mar 12, 2021
@jphickey
Fix nasa#869, rename OS_U32ValueWrapper_t
f1abc0c 
The "U32" designator in the name had become confusing because all
the members had over time migrated to a non-uint32/dedicated type.

The point is to pass a value through a void* so a name change better
indicates that purpose.
astrogeco added a commit that referenced this issue Mar 22, 2021
@astrogeco
Merge pull request #901 from jphickey/fix-869-rename-value-wrapper
468ab5e 
Fix #869, rename OS_U32ValueWrapper_t
@astrogeco astrogeco mentioned this issue Mar 22, 2021
Merged
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
@astrogeco
Merge pull request nasa#869 from CDKnightNASA/fix-769-struct_name
09219e6 
Fix nasa#769, Adds name to struct/union/enum typedefs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jphickey jphickey

Labels
CFS-38 enhancement
Projects
None yet
Milestone
Caelum
2 participants
@jphickey @skliper