pkarashchenko commented on pull request #5495: URL: https://github.com/apache/incubator-nuttx/pull/5495#issuecomment-1040133281
> Current GCC documentation states: > > > packed - This attribute, attached to a struct, union, or C++ class type definition, specifies that each of its members (other than zero-width bit-fields) is placed to minimize the memory required. This is equivalent to specifying the packed attribute on each of the members. > > So I guess @xiaoxiang781216 is right and the packed attribute on the reg_off member is not required. https://stackoverflow.com/questions/16904613/whats-the-effect-of-attribute-packed-on-nested-structs -- this is definitely not from GCC manual, but I will try to find some better proofs. The deice side use ``` begin_packed_struct struct tcbinfo_s { uint16_t pid_off; /* Offset of tcb.pid */ uint16_t state_off; /* Offset of tcb.task_state */ uint16_t pri_off; /* Offset of tcb.sched_priority */ uint16_t name_off; /* Offset of tcb.name */ uint16_t reg_num; /* Num of regs in tcbinfo.reg_offs */ /* Offset pointer of xcp.regs, order in GDB org.gnu.gdb.xxx feature. * Please refer: * https://sourceware.org/gdb/current/onlinedocs/gdb/ARM-Features.html * https://sourceware.org/gdb/current/onlinedocs/gdb/RISC_002dV-Features * -.html * value 0: This regsiter was not priovided by NuttX */ union { uint8_t u[8]; FAR const uint16_t *p; } reg_off; } end_packed_struct; ``` and for GCC we have ``` # define begin_packed_struct # define end_packed_struct __attribute__ ((packed)) ``` so let's at least keep the same style and use here ``` struct tcbinfo_s { uint16_t pid_off; uint16_t state_off; uint16_t pri_off; uint16_t name_off; uint16_t reg_num; union { uint8_t u[8]; uint16_t *p; } reg_off; uint16_t reg_offs[0]; } __attribute__ ((packed)); ``` I'm not sure why there is not `packed` attribute for `reg_off` on device side. Maybe it is not well tested on a platforms that has problems with unaligned access. I think that it is mandatory to have on both sides. You can see example in `drivers/mmcsd/sdio.c`: ``` begin_packed_struct struct sdio_resp_r5 { uint32_t data : 8; begin_packed_struct struct { uint32_t out_of_range : 1; uint32_t function_number : 1; uint32_t rfu : 1; uint32_t error : 1; uint32_t io_current_state : 2; uint32_t illegal_command : 1; uint32_t com_crc_error : 1; } end_packed_struct flags; uint32_t reserved_16 : 16; } end_packed_struct; ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
