You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/07/28 07:45:08 UTC
[GitHub] [incubator-nuttx] easonxiang opened a new pull request, #6725: include: Fixed null struct may caused build issue.
easonxiang opened a new pull request, #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725
Some compiler do not allowed null structure.
Signed-off-by: xiangdong6 <xi...@xiaomi.com>
## Summary
## Impact
## Testing
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] easonxiang closed pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
easonxiang closed pull request #6725: include: Fixed null struct may caused build issue.
URL: https://github.com/apache/incubator-nuttx/pull/6725
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#discussion_r932020193
##########
include/nuttx/spinlock.h:
##########
@@ -35,6 +35,7 @@
#ifndef CONFIG_SPINLOCK
typedef struct
{
+ uint8_t unused; /* NULL structure may caused build issue. */
Review Comment:
This would require many places add #ifdef/#endif:
```
ifdef CONFIG_SPINLOCK
struct spinlock_s xxx;
#endif
```
another approach is define the spinlock by macro like:
https://github.com/apache/incubator-nuttx/blob/master/include/nuttx/power/pm.h#L90-L91
https://github.com/apache/incubator-nuttx/blob/master/include/nuttx/power/pm.h#L802
what do you think? @masayuki2009 and @davids5 .
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] easonxiang commented on a diff in pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
easonxiang commented on code in PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#discussion_r933720681
##########
include/nuttx/net/netdev.h:
##########
@@ -258,21 +258,19 @@ struct net_driver_s
/* Link layer address */
+#if defined(CONFIG_NET_ETHERNET) || defined(CONFIG_NET_6LOWPAN) || \
+ defined(CONFIG_NET_BLUETOOTH) || defined(CONFIG_NET_IEEE802154)
union
{
-#ifdef CONFIG_NET_ETHERNET
Review Comment:
done.
##########
include/nuttx/spinlock.h:
##########
@@ -35,6 +35,7 @@
#ifndef CONFIG_SPINLOCK
typedef struct
{
+ uint8_t unused; /* NULL structure may caused build issue. */
Review Comment:
done.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#discussion_r932244273
##########
include/nuttx/spinlock.h:
##########
@@ -35,6 +35,7 @@
#ifndef CONFIG_SPINLOCK
typedef struct
{
+ uint8_t unused; /* NULL structure may caused build issue. */
Review Comment:
@easonxiang please drop the change in spinlock.h, I will provide a patch to add DECLARE_SPINLOCK macro instead.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#issuecomment-1200165419
> Why can this not be handled in the same way with Kconfig? CONFIG_NET_HAVE_DMAC is set for CONFIG_NET_ETHERNET || CONFIG_NET_6LOWPAN || CONFIG_NET_BLUETOOTH || defined(CONFIG_NET_IEEE802154 Then omit the union if CONFIG_NET_HAVE_DMAC is not set?
Yes, @easonxiang please change to:
```
#if defined(CONFIG_NET_ETHERNET) || defined(CONFIG_NET_6LOWPAN) || \
defined(CONFIG_NET_BLUETOOTH) || defined(CONFIG_NET_IEEE802154)
union
{
#ifdef CONFIG_NET_ETHERNET
/* Ethernet device identity */
struct ether_addr ether; /* Device Ethernet MAC address */
#endif
# if defined(CONFIG_NET_6LOWPAN) || defined(CONFIG_NET_BLUETOOTH) || \
defined(CONFIG_NET_IEEE802154)
/* The address assigned to an IEEE 802.15.4 or generic packet radio. */
struct netdev_varaddr_s radio;
# endif
} d_mac;
#endif
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#discussion_r932020193
##########
include/nuttx/spinlock.h:
##########
@@ -35,6 +35,7 @@
#ifndef CONFIG_SPINLOCK
typedef struct
{
+ uint8_t unused; /* NULL structure may caused build issue. */
Review Comment:
This would require many places:
```
ifdef CONFIG_SPINLOCK
struct spinlock_s xxx;
#endif
```
another approach is define the spinlock by macro like:
https://github.com/apache/incubator-nuttx/blob/master/include/nuttx/power/pm.h#L90-L91
https://github.com/apache/incubator-nuttx/blob/master/include/nuttx/power/pm.h#L802
what do you think? @masayuki2009 and @davids5 .
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#issuecomment-1200165063
> Just a note, there is no need to reproduce this. Empty struct/unions are in fact undefined behaviour according to C standard. So one way or another this should be fixed.
>
> > At least one type specifier shall be given in the declaration specifiers in each declarationand in the specifier-qualifier list in each struct declaration and type name.
@mlyszczek could you give a link in spec which state that the empty struct/union is invalid?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mlyszczek commented on pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#issuecomment-1200039940
Just a note, there is no need to reproduce this. Empty struct/unions are in fact undefined behaviour according to C standard. So one way or another this should be fixed.
> At least one type specifier shall be given in the declaration specifiers in each declarationand in the specifier-qualifier list in each struct declaration and type name.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] anchao commented on a diff in pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#discussion_r947754947
##########
include/nuttx/net/netdev.h:
##########
@@ -272,6 +272,7 @@ struct net_driver_s
struct netdev_varaddr_s radio;
#endif
+ uint8_t unused; /* it's empty structure if all above defines avoid. */
Review Comment:
Done
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] easonxiang commented on pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
easonxiang commented on PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#issuecomment-1198055034
> > Some compiler do not allowed null structure.
> > @easonxiang
>
> How can we reproduce the error?
We are trying to support Windows simulator based on visual studio. This issue can be reproduced with VS2019.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#discussion_r931923559
##########
include/nuttx/net/netdev.h:
##########
@@ -272,6 +272,7 @@ struct net_driver_s
struct netdev_varaddr_s radio;
#endif
+ uint8_t unused; /* fix compile issue if all above defines are valid */
Review Comment:
let's guard the whole union by:
```
#if defined(CONFIG_NET_ETHERNET) || defined(CONFIG_NET_6LOWPAN) || \
defined(CONFIG_NET_BLUETOOTH) || defined(CONFIG_NET_IEEE802154)
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#discussion_r932242230
##########
include/nuttx/net/netdev.h:
##########
@@ -258,21 +258,19 @@ struct net_driver_s
/* Link layer address */
+#if defined(CONFIG_NET_ETHERNET) || defined(CONFIG_NET_6LOWPAN) || \
+ defined(CONFIG_NET_BLUETOOTH) || defined(CONFIG_NET_IEEE802154)
union
{
-#ifdef CONFIG_NET_ETHERNET
Review Comment:
please keep the internal #ifdef/#endif
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#issuecomment-1200624886
```
====================================================================================
Configuration/Tool: sim/tcploop
------------------------------------------------------------------------------------
Cleaning...
Configuring...
Building NuttX...
neighbor/neighbor_lookup.c: In function 'neighbor_match':
Error: neighbor/neighbor_lookup.c:82:38: error: 'struct net_driver_s' has no member named 'd_mac'
82 | memcpy(&info->ni_laddr->u, &dev->d_mac, info->ni_laddr->na_llsize);
| ^~
make[1]: *** [Makefile:76: neighbor_lookup.o] Error 1
make[1]: Target 'libnet.a' not remade because of errors.
make: *** [tools/LibTargets.mk:65: net/libnet.a] Error 2
make: Target 'all' not remade because of errors.
/github/workspace/sources/nuttx/tools/testbuild.sh: line 290: /github/workspace/sources/nuttx/../nuttx/nuttx.manifest: No such file or directory
Normalize sim/tcploop
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a diff in pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
davids5 commented on code in PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#discussion_r931995745
##########
include/nuttx/spinlock.h:
##########
@@ -35,6 +35,7 @@
#ifndef CONFIG_SPINLOCK
typedef struct
{
+ uint8_t unused; /* NULL structure may caused build issue. */
Review Comment:
Why does this even exists? When CONFIG_SPINLOCK not defied then why is it needed? Is the proper fix to ifdef the offending code?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] mlyszczek commented on pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#issuecomment-1200227476
> > Just a note, there is no need to reproduce this. Empty struct/unions are in fact undefined behaviour according to C standard. So one way or another this should be fixed.
> > > At least one type specifier shall be given in the declaration specifiers in each declarationand in the specifier-qualifier list in each struct declaration and type name.
>
> @mlyszczek could you give a link in spec which state that the empty struct/union is invalid?
6.7.2.1.7 of ISO/IEC 9899:1999 standard
> The presence of a struct-declaration-list in a struct-or-union-specifier declares a new type, within a translation unit. The struct-declaration-list is a sequence of declarations for the members of the structure or union. **_If the struct-declaration-list contains no named members, the behavior is undefined._** The type is incomplete until after the } that terminates the list
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#discussion_r932246139
##########
include/nuttx/net/netdev.h:
##########
@@ -258,21 +258,19 @@ struct net_driver_s
/* Link layer address */
+#if defined(CONFIG_NET_ETHERNET) || defined(CONFIG_NET_6LOWPAN) || \
+ defined(CONFIG_NET_BLUETOOTH) || defined(CONFIG_NET_IEEE802154)
union
{
-#ifdef CONFIG_NET_ETHERNET
Review Comment:
yes. need to keep
```
#if defined(CONFIG_NET_6LOWPAN) || defined(CONFIG_NET_BLUETOOTH) || \
defined(CONFIG_NET_IEEE802154)
```
for `struct netdev_varaddr_s radio;`
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a diff in pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
davids5 commented on code in PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#discussion_r932143810
##########
include/nuttx/spinlock.h:
##########
@@ -35,6 +35,7 @@
#ifndef CONFIG_SPINLOCK
typedef struct
{
+ uint8_t unused; /* NULL structure may caused build issue. */
Review Comment:
@xiaoxiang781216 I would vote for DECLARE_SPINLOCK and avoid the ifdef rash
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#issuecomment-1200631360
ND protocol need mac address to do the translation between IPv6 address and MAC address, I guess some dependence miss 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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#issuecomment-1197933409
> Some compiler do not allowed null structure.
@easonxiang
How can we reproduce the error?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #6725: include: Fixed null struct may caused build issue.
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #6725:
URL: https://github.com/apache/incubator-nuttx/pull/6725#discussion_r933974303
##########
include/nuttx/net/netdev.h:
##########
@@ -272,6 +272,7 @@ struct net_driver_s
struct netdev_varaddr_s radio;
#endif
+ uint8_t unused; /* it's empty structure if all above defines avoid. */
Review Comment:
Alternate suggestion:
```
#if defined(CONFIG_NET_ETHERNET) || defined(CONFIG_NET_6LOWPAN) || \
defined(CONFIG_NET_BLUETOOTH) || defined(CONFIG_NET_IEEE802154)
union
{
#ifdef CONFIG_NET_ETHERNET
/* Ethernet device identity */
struct ether_addr ether; /* Device Ethernet MAC address */
#endif
#if defined(CONFIG_NET_6LOWPAN) || defined(CONFIG_NET_BLUETOOTH) || \
defined(CONFIG_NET_IEEE802154)
/* The address assigned to an IEEE 802.15.4 or generic packet radio. */
struct netdev_varaddr_s radio;
#endif
} d_mac;
#endif
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org