pkarashchenko commented on a change in pull request #5495:
URL: https://github.com/apache/incubator-nuttx/pull/5495#discussion_r806175553
##########
File path: tools/jlink-nuttx.c
##########
@@ -72,7 +72,7 @@ enum symbol_e
NSYMBOLS
};
-__attribute__ ((packed)) struct tcbinfo_s
+struct __attribute__ ((packed)) tcbinfo_s
Review comment:
Let's do either
```
struct tcbinfo_s __attribute__ ((packed))
```
or
```
struct tcbinfo_s
{
} __attribute__ ((packed));
```
I prefer second variant.
Also seems that
```
union
{
uint8_t u[8];
uint16_t *p;
} reg_off;
```
is missing packed attribute as well.
It should be
```
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;
} __attribute__ ((packed)) reg_off;
uint16_t reg_offs[0];
} __attribute__ ((packed));
```
##########
File path: tools/jlink-nuttx.c
##########
@@ -72,20 +91,24 @@ enum symbol_e
NSYMBOLS
};
-__attribute__ ((packed)) struct tcbinfo_s
+begin_packed_struct
+struct tcbinfo_s
{
uint16_t pid_off;
uint16_t state_off;
uint16_t pri_off;
uint16_t name_off;
uint16_t reg_num;
+ begin_packed_struct
Review comment:
Yes. I think this should be done with this PR
##########
File path: include/nuttx/sched.h
##########
@@ -781,12 +782,15 @@ begin_packed_struct struct tcbinfo_s
* value 0: This regsiter was not priovided by NuttX
*/
+ begin_packed_struct
union
{
uint8_t u[8];
FAR const uint16_t *p;
- } reg_off;
-} end_packed_struct;
+ }
+ end_packed_struct reg_off;
+}
+end_packed_struct;
Review comment:
I think this part can be kept unchanged
##########
File path: tools/jlink-nuttx.c
##########
@@ -72,20 +91,24 @@ enum symbol_e
NSYMBOLS
};
-__attribute__ ((packed)) struct tcbinfo_s
+begin_packed_struct
+struct tcbinfo_s
{
uint16_t pid_off;
uint16_t state_off;
uint16_t pri_off;
uint16_t name_off;
uint16_t reg_num;
+ begin_packed_struct
union
{
uint8_t u[8];
uint16_t *p;
- } reg_off;
+ }
+ end_packed_struct reg_off;
uint16_t reg_offs[0];
-};
+}
+end_packed_struct;
Review comment:
```suggestion
} end_packed_struct;
```
##########
File path: tools/jlink-nuttx.c
##########
@@ -72,20 +91,24 @@ enum symbol_e
NSYMBOLS
};
-__attribute__ ((packed)) struct tcbinfo_s
+begin_packed_struct
+struct tcbinfo_s
Review comment:
```suggestion
begin_packed_struct struct tcbinfo_s
```
##########
File path: include/nuttx/sched.h
##########
@@ -765,7 +765,8 @@ struct pthread_tcb_s
*/
#ifdef CONFIG_DEBUG_TCBINFO
-begin_packed_struct struct tcbinfo_s
+begin_packed_struct
+struct tcbinfo_s
Review comment:
I think this part can be kept unchanged
##########
File path: tools/jlink-nuttx.c
##########
@@ -72,20 +91,22 @@ enum symbol_e
NSYMBOLS
};
-__attribute__ ((packed)) struct tcbinfo_s
+begin_packed_struct struct tcbinfo_s
{
uint16_t pid_off;
uint16_t state_off;
uint16_t pri_off;
uint16_t name_off;
uint16_t reg_num;
+ begin_packed_struct
union
{
uint8_t u[8];
uint16_t *p;
Review comment:
I think we are missing `const` here
--
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]
