search

apache / mynewt-mcumgr

Apache mynewt
https://mynewt.apache.org/
99 stars 76 forks source link

Bit field in header #100

Open 5frank opened 3 years ago

5frank commented 3 years ago

https://github.com/apache/mynewt-mcumgr/blob/a3d5117b0888ca52b967886467b5bb350028c4ba/mgmt/include/mgmt/mgmt.h#L79-L87

I believe it is unwise to have a bit field in the header. Memory layout endianness (i.e. byte order) do not necessarily correlate with bit order and compilers are free to pick any layout and even add padding as it is not in C specification.

alokprinc commented 3 years ago

What can I do for this issue. Can you show me ropes?

5frank commented 3 years ago

@alokprinc perhaps remove bitfield and change all access to nh_op to use a bitmask. example:

struct mgmt_hdr_no_bitfield { 
    uint8_t  nh_op; /
..
#define MGMT_HDR_OP_MASK (0x7)
...
        uint8_t op = some_header.nh_op & MGMT_HDR_OP_MASK;
.
}
alokprinc commented 3 years ago 
 struct mgmt_hdr { 
 #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ 
     uint8_t  nh_op:3;           /* MGMT_OP_[...] */ 
     uint8_t  _res1:5; 
 #endif 
 #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ 
     uint8_t  _res1:5; 
     uint8_t  nh_op:3;           /* MGMT_OP_[...] */ 
 #endif

do i have to change this piece of code. And i still not getting what shold i do please guide.