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/01/04 11:54:02 UTC
[GitHub] [incubator-nuttx] Donny9 opened a new pull request #5159: socket: extend socket_storage for rpmsg_socket addrinfo
Donny9 opened a new pull request #5159:
URL: https://github.com/apache/incubator-nuttx/pull/5159
## Summary
Extend socket_storage to fix compile warning about rexecd.
rexecd.c: In function 'rexecd_main':
rexecd.c:191:9: warning: array subscript 18 is outside array bounds of 'struct sockaddr_storage[1]' [-Warray-bounds]
191 | snprintf(((FAR struct sockaddr_rpmsg *)&addr)->rp_name,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
192 | RPMSG_SOCKET_NAME_SIZE, "%d", REXECD_PORT);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
rexecd.c:142:27: note: while referencing 'addr'
142 | struct sockaddr_storage addr;
Signed-off-by: Jiuzhu Dong <do...@xiaomi.com>
## Impact
fix compile warning
## Testing
daily test
--
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] Donny9 edited a comment on pull request #5159: socket: extend socket_storage for rpmsg_socket addrinfo
Posted by GitBox <gi...@apache.org>.
Donny9 edited a comment on pull request #5159:
URL: https://github.com/apache/incubator-nuttx/pull/5159#issuecomment-1005350785
> NTP daemon
@masayuki2009 Okay, you can try this PR https://github.com/apache/incubator-nuttx-apps/pull/957, we can optimize this stack used for ntp daemon. thank you!
--
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 #5159: socket: extend socket_storage for rpmsg_socket addrinfo
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5159:
URL: https://github.com/apache/incubator-nuttx/pull/5159
--
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] Donny9 commented on pull request #5159: socket: extend socket_storage for rpmsg_socket addrinfo
Posted by GitBox <gi...@apache.org>.
Donny9 commented on pull request #5159:
URL: https://github.com/apache/incubator-nuttx/pull/5159#issuecomment-1005350785
> NTP daemon
okay, you can try this PR https://github.com/apache/incubator-nuttx-apps/pull/957, we can optimize this stack used for ntp daemon.
--
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 change in pull request #5159: socket: extend socket_storage for rpmsg_socket addrinfo
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5159:
URL: https://github.com/apache/incubator-nuttx/pull/5159#discussion_r778172599
##########
File path: include/sys/socket.h
##########
@@ -284,23 +284,13 @@
* aligned at an appropriate boundary so that pointers to it can be cast
* as pointers to protocol-specific address structures and used to access
* the fields of those structures without alignment problems.
- *
- * REVISIT: sizeof(struct sockaddr_storge) should be 128 bytes.
*/
-#ifdef CONFIG_NET_IPv6
struct sockaddr_storage
{
sa_family_t ss_family; /* Address family */
- char ss_data[26]; /* 26-bytes of address data */
+ char ss_data[126]; /* 126-bytes of address data */
Review comment:
I see. Thank you!
--
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 change in pull request #5159: socket: extend socket_storage for rpmsg_socket addrinfo
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5159:
URL: https://github.com/apache/incubator-nuttx/pull/5159#discussion_r778168069
##########
File path: include/sys/socket.h
##########
@@ -284,23 +284,13 @@
* aligned at an appropriate boundary so that pointers to it can be cast
* as pointers to protocol-specific address structures and used to access
* the fields of those structures without alignment problems.
- *
- * REVISIT: sizeof(struct sockaddr_storge) should be 128 bytes.
*/
-#ifdef CONFIG_NET_IPv6
struct sockaddr_storage
{
sa_family_t ss_family; /* Address family */
- char ss_data[26]; /* 26-bytes of address data */
+ char ss_data[126]; /* 126-bytes of address data */
Review comment:
Since sockaddr_un is 110 bytes:
```
#define UNIX_PATH_MAX 108
struct sockaddr_un
{
sa_family_t sun_family; /* AF_UNIX */
char sun_path[UNIX_PATH_MAX]; /* pathname */
};
```
Per the spec, we have to extend to 110 at least which is close to 128 anyway.:(
--
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 change in pull request #5159: socket: extend socket_storage for rpmsg_socket addrinfo
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5159:
URL: https://github.com/apache/incubator-nuttx/pull/5159#discussion_r778168069
##########
File path: include/sys/socket.h
##########
@@ -284,23 +284,13 @@
* aligned at an appropriate boundary so that pointers to it can be cast
* as pointers to protocol-specific address structures and used to access
* the fields of those structures without alignment problems.
- *
- * REVISIT: sizeof(struct sockaddr_storge) should be 128 bytes.
*/
-#ifdef CONFIG_NET_IPv6
struct sockaddr_storage
{
sa_family_t ss_family; /* Address family */
- char ss_data[26]; /* 26-bytes of address data */
+ char ss_data[126]; /* 126-bytes of address data */
Review comment:
Since sockaddr_un is 110 bytes:
```
#define UNIX_PATH_MAX 108
struct sockaddr_un
{
sa_family_t sun_family; /* AF_UNIX */
char sun_path[UNIX_PATH_MAX]; /* pathname */
};
```
Per the spec, we have to extend to 110 which is close to 128 anyway.:(
--
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 change in pull request #5159: socket: extend socket_storage for rpmsg_socket addrinfo
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5159:
URL: https://github.com/apache/incubator-nuttx/pull/5159#discussion_r778189089
##########
File path: include/sys/socket.h
##########
@@ -284,23 +284,13 @@
* aligned at an appropriate boundary so that pointers to it can be cast
* as pointers to protocol-specific address structures and used to access
* the fields of those structures without alignment problems.
- *
- * REVISIT: sizeof(struct sockaddr_storge) should be 128 bytes.
*/
-#ifdef CONFIG_NET_IPv6
struct sockaddr_storage
{
sa_family_t ss_family; /* Address family */
- char ss_data[26]; /* 26-bytes of address data */
+ char ss_data[126]; /* 126-bytes of address data */
Review comment:
@pkarashchenko do you finish setup apache account? If so, you could merge the PR after you finish review for the simple change like this:).
--
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 #5159: socket: extend socket_storage for rpmsg_socket addrinfo
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #5159:
URL: https://github.com/apache/incubator-nuttx/pull/5159#issuecomment-1005267419
@Donny9 @xiaoxiang781216
Hmm, I noticed that the NTP daemon with spresense:wifi_smp crashed due to stack corruption.
Before this PR,
```
24 24 --- 100 RR Task --- Waiting Signal 00000000 001976 001708 86.4%! NTP daemon 0.pool.ntp.org;1.pool.ntp.org;
```
After this PR,
```
[ 5.144101] [CPU0] arm_dump_task: 24 100 1976 1976 100.0%! NTP daemon
```
If I increase the stack size, it works but it needs much memory (almost double).
What do you think?
```
24 24 --- 100 RR Task --- Waiting Signal 00000000 004024 003052 75.8% NTP daemon 0.pool.ntp.org;1.pool.ntp.org;
```
--
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 change in pull request #5159: socket: extend socket_storage for rpmsg_socket addrinfo
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5159:
URL: https://github.com/apache/incubator-nuttx/pull/5159#discussion_r778393269
##########
File path: include/sys/socket.h
##########
@@ -284,23 +284,13 @@
* aligned at an appropriate boundary so that pointers to it can be cast
* as pointers to protocol-specific address structures and used to access
* the fields of those structures without alignment problems.
- *
- * REVISIT: sizeof(struct sockaddr_storge) should be 128 bytes.
*/
-#ifdef CONFIG_NET_IPv6
struct sockaddr_storage
{
sa_family_t ss_family; /* Address family */
- char ss_data[26]; /* 26-bytes of address data */
+ char ss_data[126]; /* 126-bytes of address data */
Review comment:
> @pkarashchenko do you finish setup apache account? If so, you could merge the PR after you finish review for the simple change like this:).
It's on the way :) I'll join team soon.
--
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 change in pull request #5159: socket: extend socket_storage for rpmsg_socket addrinfo
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5159:
URL: https://github.com/apache/incubator-nuttx/pull/5159#discussion_r778104011
##########
File path: include/sys/socket.h
##########
@@ -284,23 +284,13 @@
* aligned at an appropriate boundary so that pointers to it can be cast
* as pointers to protocol-specific address structures and used to access
* the fields of those structures without alignment problems.
- *
- * REVISIT: sizeof(struct sockaddr_storge) should be 128 bytes.
*/
-#ifdef CONFIG_NET_IPv6
struct sockaddr_storage
{
sa_family_t ss_family; /* Address family */
- char ss_data[26]; /* 26-bytes of address data */
+ char ss_data[126]; /* 126-bytes of address data */
Review comment:
`126` is quite expensive. Maybe we can change to 32 to handle `rexecd` and extend in case if it will be needed in future?
##########
File path: include/sys/socket.h
##########
@@ -284,23 +284,13 @@
* aligned at an appropriate boundary so that pointers to it can be cast
* as pointers to protocol-specific address structures and used to access
* the fields of those structures without alignment problems.
- *
- * REVISIT: sizeof(struct sockaddr_storge) should be 128 bytes.
*/
-#ifdef CONFIG_NET_IPv6
struct sockaddr_storage
{
sa_family_t ss_family; /* Address family */
- char ss_data[26]; /* 26-bytes of address data */
+ char ss_data[126]; /* 126-bytes of address data */
Review comment:
`126` is quite expensive. Maybe we can change to `32` to handle `rexecd` and extend in case if it will be needed in future?
--
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