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 2021/06/14 14:54:43 UTC
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 opened a new pull request #770: system/ping[6]: correct the ping return value
xiaoxiang781216 opened a new pull request #770:
URL: https://github.com/apache/incubator-nuttx-apps/pull/770
## Summary
## Impact
Return EXIT_FAILURE in the error case
## 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] yamt commented on a change in pull request #770: system/ping[6]: correct the ping return value
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #770:
URL: https://github.com/apache/incubator-nuttx-apps/pull/770#discussion_r655077732
##########
File path: system/ping/ping.c
##########
@@ -224,6 +241,8 @@ int main(int argc, FAR char *argv[])
info.delay = ICMP_POLL_DELAY;
info.timeout = ICMP_POLL_DELAY;
info.callback = ping_result;
+ info.priv = &priv;
+ priv.code = ICMP_I_BEGIN;
Review comment:
i feel it's cleaner to introduce a dedicated constant than using ICMP_I_BEGIN
--
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
[GitHub] [incubator-nuttx-apps] yamt commented on a change in pull request #770: system/ping[6]: correct the ping return value
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #770:
URL: https://github.com/apache/incubator-nuttx-apps/pull/770#discussion_r655042425
##########
File path: system/ping6/ping6.c
##########
@@ -92,8 +101,11 @@ static void show_usage(FAR const char *progname, int exitcode)
static void ping6_result(FAR const struct ping6_result_s *result)
{
+ FAR struct ping6_priv_s *priv = result->info->priv;
char strbuffer[INET6_ADDRSTRLEN];
+ priv->code = result->code;
Review comment:
i guess this way many of errors will be overwritten by ICMP_I_FINISH.
is it intended?
--
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
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #770: system/ping[6]: correct the ping return value
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #770:
URL: https://github.com/apache/incubator-nuttx-apps/pull/770#discussion_r655094692
##########
File path: system/ping/ping.c
##########
@@ -224,6 +241,8 @@ int main(int argc, FAR char *argv[])
info.delay = ICMP_POLL_DELAY;
info.timeout = ICMP_POLL_DELAY;
info.callback = ping_result;
+ info.priv = &priv;
+ priv.code = ICMP_I_BEGIN;
Review comment:
Fix
--
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
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #770: system/ping[6]: correct the ping return value
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #770:
URL: https://github.com/apache/incubator-nuttx-apps/pull/770#discussion_r655062114
##########
File path: system/ping6/ping6.c
##########
@@ -92,8 +101,11 @@ static void show_usage(FAR const char *progname, int exitcode)
static void ping6_result(FAR const struct ping6_result_s *result)
{
+ FAR struct ping6_priv_s *priv = result->info->priv;
char strbuffer[INET6_ADDRSTRLEN];
+ priv->code = result->code;
Review comment:
Yes, you are right. Please review the updated version again.
--
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
[GitHub] [incubator-nuttx-apps] acassis merged pull request #770: system/ping[6]: correct the ping return value
Posted by GitBox <gi...@apache.org>.
acassis merged pull request #770:
URL: https://github.com/apache/incubator-nuttx-apps/pull/770
--
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