Byte alignment in coap_context_t？

[gli-spirent]

There are some byte aligment issue in coap_context_t. The two below are defined in the middle of the structure, which would introduce more space by this structure. uint8_t timer_configured; /**< Set to 1 when a retransmission is uint16_t message_id;

I did a test on x86_64, the size of coap_context_t is 232. It would be 224 if I moved the two to the line under the line as below: unsigned int ping_timeout; /< Minimum inactivity time before sending a ping message. 0 means disabled. */ unsigned int csm_timeout; /< Timeout for waiting for a CSM from the remote side. 0 means disabled. */

It would matter in a large scale.

[obgm]

While I do not see how 8 bytes would matter at large scale (the context is used only one for an application), I do not see harm in your proposed change. @mrdeep1 Do you have objections to this?

[mrdeep1]

It would be worth a review of all structures to see if there is dead space that can be utilized - especially as x86_64 aligns things like pointers to 8 byte boundaries.

However, re-organizing a structure make an updated library incompatible with any applications built against the previous structure definitions - especially as the structures such as coap_context_t, coap_session_t etc. are not opaque to an application. Would it be acceptable for a rebuild of all the applications as well if this change is implemented?

[gli-spirent]

@obgm I did not go through every struct listed in context. Another example is coap_async_state_t You can see a 1-byte flags at the beginning followed with a 8 byte(x86_64) tick_t. And a 2 bytes message id between 2 pointers. And a 4 bytes tid_t. There would be totally(x86_64) 7+6+4=17 bytes wasted in this structure. And may be more for the context because there is a list of coap_async_state_t in case. typedef struct coap_async_state_t { unsigned char flags; /**< holds the flags to control behaviour */

/**

• Holds the internal time when the object was registered with a
• resource. This field will be updated whenever
• coap_register_async() is called for a specific resource. */ coap_tick_t created;

/**

• This field can be used to register opaque application data with the
• asynchronous state object. */ void *appdata; uint16_t message_id; /< id of last message seen */ coap_session_t *session; /< transaction session */ coap_tid_t id; /< transaction id */ struct coap_async_state_t *next; /< internally used for linking */ size_t tokenlen; /< length of the token */ uint8_t token[8]; /< the token to use in a response */ } coap_async_state_t;

[gli-spirent]

Most structures used in coap_context_t have the issue. coap_queue_t coap_session_t coap_endpoint_t(?)

[obgm]

@gli-spirent Understood. You have a very good point here. As @mrdeep1 correctly points out, changing this has some implications on the existing applications that link against libcoap. As version 4.2.0 is currently being packaged for Debian, I suppose this needs to go into the next major API revision.

[gli-spirent]

@obgm @mrdeep1 Yes, it's good to consider this in future development. Thanks a lot! BTW: does libcoap have limitation on the resources? I mean maximum URI can be added? Is 3000K OK? I'm still investigating the code and did not find the hash bucket size yet. Do you have a quick answer?

[obgm]

In uthash.h, the initial hash bucket size is defined as:

#define HASH_INITIAL_NUM_BUCKETS 32U

HASH_ADD eventually calls HASH_EXPAND_BUCKETS which may or may not grow the hash table (I did not follow all conditions that are checked). You can define uthash_expand_fyi and uthash_expand_fyi to get noticed when this happens.

[gli-spirent]

Data before and after ajustment coap_context_t is 232 sizeof(coap_queue_t)= 48 sizeof(coap_async_state_t)= 72 sizeof(coap_session_t)= 312 sizeof(coap_endpoint_t)= 384 sizeof(coap_resource_t)= 160

coap_context_t would be 224 sizeof(coap_queue_t)= 48 sizeof(coap_async_state_t)= 56 sizeof(coap_session_t)= 304 sizeof(coap_endpoint_t)= 376 sizeof(coap_resource_t)= 160

[obgm]

Thanks for collecting these numbers. If you do not mind, would you please open a PR with the corresponding changes so they do not get lost?

[gli-spirent]

Will do it.

[obgm]

Which is now PR #315 — thank you!