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/12/15 08:36:22 UTC
[GitHub] [incubator-nuttx] takumiando opened a new pull request #5004: libc/unistd: getopt: Use argc to end parsing
takumiando opened a new pull request #5004:
URL: https://github.com/apache/incubator-nuttx/pull/5004
It should end parsing with argc even the argv are remaining or it may access to invalid address.
--
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] takumiando commented on a change in pull request #5004: libc/unistd: getopt: Use argc to end parsing
Posted by GitBox <gi...@apache.org>.
takumiando commented on a change in pull request #5004:
URL: https://github.com/apache/incubator-nuttx/pull/5004#discussion_r769419884
##########
File path: libs/libc/unistd/lib_getopt_common.c
##########
@@ -393,6 +393,14 @@ int getopt_common(int argc, FAR char * const argv[],
/* Check for the end of the argument list */
+ if (go->go_optind >= argc)
+ {
+ /* We don't need to check arguments anymore, we are finished */
+
+ go->go_binitialized = false;
+ return ERROR;
+ }
+
go->go_optptr = argv[go->go_optind];
Review comment:
Is ternary operator recommended if it can be shorter or simple in NuttX project ?
I sent PR to here for the first time.
--
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 #5004: libc/unistd: getopt: Use argc to end parsing
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5004:
URL: https://github.com/apache/incubator-nuttx/pull/5004
--
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 #5004: libc/unistd: getopt: Use argc to end parsing
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5004:
URL: https://github.com/apache/incubator-nuttx/pull/5004#discussion_r769410879
##########
File path: libs/libc/unistd/lib_getopt_common.c
##########
@@ -393,6 +393,14 @@ int getopt_common(int argc, FAR char * const argv[],
/* Check for the end of the argument list */
+ if (go->go_optind >= argc)
+ {
+ /* We don't need to check arguments anymore, we are finished */
+
+ go->go_binitialized = false;
+ return ERROR;
+ }
+
go->go_optptr = argv[go->go_optind];
Review comment:
```suggestion
go->go_optptr = go->go_optind < argc ? argv[go->go_optind] : NULL;
```
--
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 #5004: libc/unistd: getopt: Use argc to end parsing
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5004:
URL: https://github.com/apache/incubator-nuttx/pull/5004#discussion_r769390280
##########
File path: libs/libc/unistd/lib_getopt_common.c
##########
@@ -394,7 +394,7 @@ int getopt_common(int argc, FAR char * const argv[],
/* Check for the end of the argument list */
go->go_optptr = argv[go->go_optind];
- if (!go->go_optptr)
+ if (!go->go_optptr || go->go_optind >= argc)
Review comment:
maybe better to check `go->go_optind >= argc` before accessing `argv[go->go_optind]` above?
--
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 #5004: libc/unistd: getopt: Use argc to end parsing
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5004:
URL: https://github.com/apache/incubator-nuttx/pull/5004#discussion_r769411130
##########
File path: libs/libc/unistd/lib_getopt_common.c
##########
@@ -393,6 +393,14 @@ int getopt_common(int argc, FAR char * const argv[],
/* Check for the end of the argument list */
+ if (go->go_optind >= argc)
+ {
+ /* We don't need to check arguments anymore, we are finished */
+
+ go->go_binitialized = false;
+ return ERROR;
+ }
+
go->go_optptr = argv[go->go_optind];
Review comment:
Just as alternative shorter solution to get the same result
--
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] takumiando commented on a change in pull request #5004: libc/unistd: getopt: Use argc to end parsing
Posted by GitBox <gi...@apache.org>.
takumiando commented on a change in pull request #5004:
URL: https://github.com/apache/incubator-nuttx/pull/5004#discussion_r769407183
##########
File path: libs/libc/unistd/lib_getopt_common.c
##########
@@ -394,7 +394,7 @@ int getopt_common(int argc, FAR char * const argv[],
/* Check for the end of the argument list */
go->go_optptr = argv[go->go_optind];
- if (!go->go_optptr)
+ if (!go->go_optptr || go->go_optind >= argc)
Review comment:
@pkarashchenko
Thanks for your review!
You're right, I force-pushed 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.
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 #5004: libc/unistd: getopt: Use argc to end parsing
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5004:
URL: https://github.com/apache/incubator-nuttx/pull/5004#discussion_r769449261
##########
File path: libs/libc/unistd/lib_getopt_common.c
##########
@@ -393,6 +393,14 @@ int getopt_common(int argc, FAR char * const argv[],
/* Check for the end of the argument list */
+ if (go->go_optind >= argc)
+ {
+ /* We don't need to check arguments anymore, we are finished */
+
+ go->go_binitialized = false;
+ return ERROR;
+ }
+
go->go_optptr = argv[go->go_optind];
Review comment:
I'm not sure how to answer your question. In general ternary operator is widely used in NuttX code. Also I think that code duplication should be avoided and if we can achieve this with ternary operator then it should be used.
--
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 #5004: libc/unistd: getopt: Use argc to end parsing
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5004:
URL: https://github.com/apache/incubator-nuttx/pull/5004#issuecomment-995034073
Let's ignore the CI setup error:
fatal: unable to access 'https://bitbucket.org/nuttx/tools.git/': Failed to connect to bitbucket.org port 443: Operation timed out
--
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