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 2020/02/21 06:35:18 UTC
[GitHub] [incubator-nuttx-apps] masayuki2009 opened a new pull request #85:
Fix usrsocktest errors
masayuki2009 opened a new pull request #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85
### Summary
- This PR fixes a part of usrsocktest errors. (i.e. errors in NoBlockRecv and BlockRecv)
- Actually, the test cases checked sockaddr_in values which contain padding field, however, the field should be ignored because it contains random values.
### Impact
- This PR affects a usrsocktest application only.
### Limitations / TODO
- There still remain errors and need more investigation.
### Testing
- I tested this PR with spresense:wifi which I added usrsocktest application.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] lukegluke commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
lukegluke commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589555787
> But I think the requirement is the application should zero out the input addr before pass to socket API to avoid the undefined behaviour? addr is an output argument in our case.
It was the first thing I did trying fix it - clear input addr in usrsocktest before pass to socket API, but netstack insert random values in sin_zero itself 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589642866
@lukegluke
>I'm too far from an expert in net stack, and don't know about any specification,
>but maybe (just a thought) it would be more correct to nulling padding somewhere
>inside the net stack?
As far as I checked the Linux kernel source code, you are right.
I think zero padding should be implemented in net/socket/recvfrom.c,
if the from address family is AF_INET and fromlen is larger than sockaddr_in size.
Anyway, I'll try to implement it and then should be reviewed by @patacongo.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589548425
@lukegluke
>By the way, when I was investigating this I encountered on this sentence in stackoverflow:
Thanks for the comment. I'll check the article.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589570472
@lukegluke
>It was the first thing I did trying fix it - clear input addr in usrsocktest before pass to socket API, >but netstack insert random values in sin_zero itself anyway.
Actually recvfrom_request() in usrsocktest_daemon.c sets the return values,
so we need to modify the above function as well, if we do not modify
network stack 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589652069
@xiaoxiang781216
>If so other similar function accept, getsockname getpeername... need the similar change too
Thanks for the comments. I understand.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589524387
@xiaoxiang781216 Sorry, I've just pushed with -f
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-590612331
I've just sent a new PR. https://github.com/apache/incubator-nuttx/pull/370
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] lukegluke commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
lukegluke commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589542897
It fix fails in NoBlockRecv and BlockRecv, yes, thanks!
By the way, when I was investigating this I encountered on this sentence in [stackoverflow](https://stackoverflow.com/questions/15608707/why-is-zero-padding-needed-in-sockaddr-in):
> Its required by specification to clear sin_zero
I'm too far from an expert in net stack, and don't know about any specification, but maybe (just a thought) it would be more correct to nulling padding somewhere inside the net stack?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 merged pull request #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 edited a comment on issue
#85: Fix usrsocktest errors
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589524643
@masayuki2009 no problem, I have merged it, thanks for your patch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589520656
@masayuki2009 how about remove Make.defs patch from this PR? so I can merge it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589646309
If so other similar function accept, getsockname getpeername... need the similar change too
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589551503
But I think the requirement is the application should zero out the input addr before pass to socket API to avoid the undefined behaviour? addr is an output argument in our case.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589522738
@xiaoxiang781216 sorry, I added the file by mistake.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on issue #85: Fix
usrsocktest errors
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #85: Fix usrsocktest errors
URL: https://github.com/apache/incubator-nuttx-apps/pull/85#issuecomment-589524643
@masayuki2009 no problem, I have merged it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services