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

Correct use of "MemAddress" and "MemOffset" #2240

Closed
jphickey opened this issue Feb 2, 2023 · 0 comments · Fixed by #2256
Closed

Correct use of "MemAddress" and "MemOffset" #2240

jphickey opened this issue Feb 2, 2023 · 0 comments · Fixed by #2256

Comments

@jphickey
Copy link
Contributor

jphickey commented Feb 2, 2023

Is your feature request related to a problem? Please describe.
The MemAddress and MemOffset types defined by ES are 32 bits by default, but are intended to be expandable to 64. These should be used for CMD/TLM fields that need to hold a memory address on a 64-bit machine, or the size of an object in memory on such a machine.

However TBL services uses this type in some other locations, that generally do not need it. Notably, this calls 32-bit byte swapping routines to read/write this field, and that will not work if the size becomes 64 bits.

Describe the solution you'd like

  • These fields in table services will need to remain 32 bits regardless of the CPU address size.
  • Need to make sure that the correct macro - CFE_ES_MEMOFFSET_C or CFE_ES_MEMADDRESS_C is used when setting the value. Notably, this macro can provide the correct read/write logic in case the MemAddress needs to be a struct with a high and low word, for example.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Mar 13, 2023
dzbaker added a commit that referenced this issue Mar 16, 2023
Fix #2240, improve 64-bit memory address handling in CMD/TLM
@dmknutsen dmknutsen added this to the Equuleus milestone May 26, 2023
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.

3 participants