-
Notifications
You must be signed in to change notification settings - Fork 45
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 should use larger cf_entity_id_t type as default #62
Comments
I think this one is worth changing the default entity ID size from 8 to 32 bits. There are a couple reasons:
Because it is not simply a platform config, but also affects commands - this is why it's better to err on the side of using the larger size. If the user only really needs 8 bits, of addressing, but the config is set to 32 bits, everything should still work, it only costs a bit of memory (and a fairly trivial amount, really). And users who care about that can do the work to reduce it back to 8.. But if the user needs >8 bits this can cause some pretty confusing issues, particularly on the receive side if the sender happens to encode the entity IDs in >8 bits and CF rejects it. |
Entity IDs should be larger by default for real-world applicability, as this also restricts what CFDP can receive, not just what it sends. Note CFDP will only use the number of bytes required to express the value, so values less than 256 will still only use 1 byte, regardless of this config. This just allows use of larger values.
Entity IDs should be larger by default for real-world applicability, as this also restricts what CFDP can receive, not just what it sends. Note CFDP will only use the number of bytes required to express the value, so values less than 256 will still only use 1 byte, regardless of this config. This just allows use of larger values.
Fix #62, make entity ID default to 32 bit
This issue was imported from the GSFC issue tracking system
Imported from: [GSFCCFS-1799] CF should use larger cf_entity_id_t type as default
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Wed Nov 17 10:10:01 2021
Original Description:
In the default configuration, the cf_entity_id_t is defined as a uint8. While this is permissible per the spec, it means none of the "variable size" features implemented in the code will ever be used, because the size is always 1.
In particular the "CF_GetMemcpySize" function will only ever be called with a size of 1, which basically skips the loops in here.
Although the test do call it with a bigger sizes (200!!?!) it does not seem to actually check the intended purpose of this routine, and the tests only appear to be included when ENDIAN==_EL.
The text was updated successfully, but these errors were encountered: