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/15 19:09:00 UTC
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 opened a new pull request #778: example/posix_spawn: Ensure argv has filename and NULL terminator
xiaoxiang781216 opened a new pull request #778:
URL: https://github.com/apache/incubator-nuttx-apps/pull/778
## Summary
Report here: https://github.com/apache/incubator-nuttx/pull/3928
## Impact
Fix the non-standard usage.
## 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] xiaoxiang781216 commented on pull request #778: example/posix_spawn: Ensure argv has filename and NULL terminator
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #778:
URL: https://github.com/apache/incubator-nuttx-apps/pull/778#issuecomment-862025315
> Won't your change will fail in these cases:
>
> examples/elf/elf_main.c:
>
> ```
> args[0] = NULL;
> ret = exec(filename, args, g_elf_exports, g_elf_nexports);
> ```
>
Done.
> examples/nxflat/nxflat_main.c:
>
> ```
> args[0] = NULL;
> ret = exec(filename, args, g_nxflat_exports, g_nxflat_nexports);
> ```
>
Done.
> netutils/thttpd/thttpd_cgi.c:
>
> ```
> #ifdef CONFIG_THTTPD_NXFLAT
> child = exec(hc->expnfilename, (FAR char * const *)argp,
> g_thttpdsymtab, g_thttpdnsymbols);
> #else
> child = exec(hc->expnfilename, (FAR char * const *)argp, NULL, 0);
> #endif
> ```
>
argp contain filename arleady, we don't need change:
https://github.com/apache/incubator-nuttx-apps/blob/d6d458c60be20d1cc4954552f66989330d63a0ee/netutils/thttpd/thttpd_cgi.c#L323-L331
> In the above, argv[0] is NOT the file name but argv will not be NULL either.
>
> Hmm.. the first two should be okay because you check (argv && argv[0]), but I am not sure about the last one. That check, (argv && argv[0]), is NOT foolproof.
--
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 #778: example/posix_spawn: Ensure argv has filename and NULL terminator
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #778:
URL: https://github.com/apache/incubator-nuttx-apps/pull/778#discussion_r652336098
##########
File path: examples/posix_spawn/spawn_main.c
##########
@@ -117,16 +117,20 @@ static unsigned int g_mmstep; /* Memory Usage at beginning of test step */
static const char delimiter[] =
"**************************************"
"**************************************";
-static const char g_redirect[] = "redirect";
static const char g_data[] = "testdata.txt";
static char fullpath[128];
-static char * const g_argv[5] =
+static char * const g_hello_argv[5] =
{
"hello", "Argument 1", "Argument 2", "Argument 3", NULL
};
+static char * const g_redirect_argv[5] =
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull request #778: example/posix_spawn: Ensure argv has filename and NULL terminator
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #778:
URL: https://github.com/apache/incubator-nuttx-apps/pull/778#discussion_r652108625
##########
File path: examples/posix_spawn/spawn_main.c
##########
@@ -117,16 +117,20 @@ static unsigned int g_mmstep; /* Memory Usage at beginning of test step */
static const char delimiter[] =
"**************************************"
"**************************************";
-static const char g_redirect[] = "redirect";
static const char g_data[] = "testdata.txt";
static char fullpath[128];
-static char * const g_argv[5] =
+static char * const g_hello_argv[5] =
{
"hello", "Argument 1", "Argument 2", "Argument 3", NULL
};
+static char * const g_redirect_argv[5] =
Review comment:
```suggestion
static char * const g_redirect_argv[2] =
```
--
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] patacongo commented on pull request #778: example/posix_spawn: Ensure argv has filename and NULL terminator
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #778:
URL: https://github.com/apache/incubator-nuttx-apps/pull/778#issuecomment-861818892
Won't your change will fail in these cases:
examples/elf/elf_main.c:
args[0] = NULL;
ret = exec(filename, args, g_elf_exports, g_elf_nexports);
examples/nxflat/nxflat_main.c:
args[0] = NULL;
ret = exec(filename, args, g_nxflat_exports, g_nxflat_nexports);
netutils/thttpd/thttpd_cgi.c:
#ifdef CONFIG_THTTPD_NXFLAT
child = exec(hc->expnfilename, (FAR char * const *)argp,
g_thttpdsymtab, g_thttpdnsymbols);
#else
child = exec(hc->expnfilename, (FAR char * const *)argp, NULL, 0);
#endif
In the above, argv[0] is NOT the file name but argv will not be NULL either.
--
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] patacongo edited a comment on pull request #778: example/posix_spawn: Ensure argv has filename and NULL terminator
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #778:
URL: https://github.com/apache/incubator-nuttx-apps/pull/778#issuecomment-861818892
Won't your change will fail in these cases:
examples/elf/elf_main.c:
args[0] = NULL;
ret = exec(filename, args, g_elf_exports, g_elf_nexports);
examples/nxflat/nxflat_main.c:
args[0] = NULL;
ret = exec(filename, args, g_nxflat_exports, g_nxflat_nexports);
netutils/thttpd/thttpd_cgi.c:
#ifdef CONFIG_THTTPD_NXFLAT
child = exec(hc->expnfilename, (FAR char * const *)argp,
g_thttpdsymtab, g_thttpdnsymbols);
#else
child = exec(hc->expnfilename, (FAR char * const *)argp, NULL, 0);
#endif
In the above, argv[0] is NOT the file name but argv will not be NULL either.
Hmm.. the first two should be okay because you check (argv && argv[0]), but I am not sure about the last one. That check, (argv && argv[0]), is NOT foolproof.
--
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