You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by ma...@apache.org on 2021/06/02 06:20:49 UTC
[incubator-nuttx] 01/02: binfmt: Move argv copy into exec_module
This is an automated email from the ASF dual-hosted git repository.
masayuki pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit cf78a5b6cfb7b6f06c8d229f5083c5aac82700ee
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Mon May 24 15:06:29 2021 +0800
binfmt: Move argv copy into exec_module
and remove the related fields from struct binary_s
Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
---
binfmt/binfmt.h | 11 +++++-----
binfmt/binfmt_copyargv.c | 51 ++++++++++++++++++-------------------------
binfmt/binfmt_dumpmodule.c | 1 -
binfmt/binfmt_exec.c | 16 ++------------
binfmt/binfmt_execmodule.c | 27 +++++++++++++----------
binfmt/binfmt_unloadmodule.c | 4 ----
include/nuttx/binfmt/binfmt.h | 8 +------
7 files changed, 44 insertions(+), 74 deletions(-)
diff --git a/binfmt/binfmt.h b/binfmt/binfmt.h
index 011e7c3..2d4b7d9 100644
--- a/binfmt/binfmt.h
+++ b/binfmt/binfmt.h
@@ -88,15 +88,14 @@ int binfmt_dumpmodule(FAR const struct binary_s *bin);
* do not have any real option other than to copy the callers argv[] list.
*
* Input Parameters:
- * bin - Load structure
* argv - Argument list
*
* Returned Value:
- * Zero (OK) on success; a negated errno value on failure.
+ * A non-zero copy is returned on success.
*
****************************************************************************/
-int binfmt_copyargv(FAR struct binary_s *bin, FAR char * const *argv);
+FAR char * const *binfmt_copyargv(FAR char * const *argv);
/****************************************************************************
* Name: binfmt_freeargv
@@ -105,7 +104,7 @@ int binfmt_copyargv(FAR struct binary_s *bin, FAR char * const *argv);
* Release the copied argv[] list.
*
* Input Parameters:
- * bin - Load structure
+ * argv - Argument list
*
* Returned Value:
* None
@@ -113,9 +112,9 @@ int binfmt_copyargv(FAR struct binary_s *bin, FAR char * const *argv);
****************************************************************************/
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
-void binfmt_freeargv(FAR struct binary_s *bin);
+void binfmt_freeargv(FAR char * const *argv);
#else
-# define binfmt_freeargv(bin)
+# define binfmt_freeargv(argv)
#endif
/****************************************************************************
diff --git a/binfmt/binfmt_copyargv.c b/binfmt/binfmt_copyargv.c
index af429d5..352f254 100644
--- a/binfmt/binfmt_copyargv.c
+++ b/binfmt/binfmt_copyargv.c
@@ -59,17 +59,17 @@
* do not have any real option other than to copy the callers argv[] list.
*
* Input Parameters:
- * bin - Load structure
* argv - Argument list
*
* Returned Value:
- * Zero (OK) on success; a negated error value on failure.
+ * A non-zero copy is returned on success.
*
****************************************************************************/
-int binfmt_copyargv(FAR struct binary_s *bin, FAR char * const *argv)
+FAR char * const *binfmt_copyargv(FAR char * const *argv)
{
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
+ FAR char **argvbuf = NULL;
FAR char *ptr;
size_t argvsize;
size_t argsize;
@@ -78,9 +78,6 @@ int binfmt_copyargv(FAR struct binary_s *bin, FAR char * const *argv)
/* Get the number of arguments and the size of the argument list */
- bin->argv = (FAR char **)NULL;
- bin->argbuffer = (FAR char *)NULL;
-
if (argv)
{
argsize = 0;
@@ -105,7 +102,7 @@ int binfmt_copyargv(FAR struct binary_s *bin, FAR char * const *argv)
{
berr("ERROR: Too many arguments: %lu\n",
(unsigned long)argvsize);
- return -E2BIG;
+ return NULL;
}
}
@@ -115,39 +112,38 @@ int binfmt_copyargv(FAR struct binary_s *bin, FAR char * const *argv)
if (argsize > 0)
{
- argvsize = (nargs + 1) * sizeof(FAR char *);
- bin->argbuffer = (FAR char *)kmm_malloc(argvsize + argsize);
- if (!bin->argbuffer)
+ argvsize = (nargs + 1) * sizeof(FAR char *);
+ ptr = (FAR char *)kmm_malloc(argvsize + argsize);
+ if (!ptr)
{
berr("ERROR: Failed to allocate the argument buffer\n");
- return -ENOMEM;
+ return NULL;
}
/* Copy the argv list */
- bin->argv = (FAR char **)bin->argbuffer;
- ptr = bin->argbuffer + argvsize;
+ argvbuf = (FAR char **)ptr;
+ ptr += argvsize;
for (i = 0; argv[i]; i++)
{
- bin->argv[i] = ptr;
- argsize = strlen(argv[i]) + 1;
+ argvbuf[i] = ptr;
+ argsize = strlen(argv[i]) + 1;
memcpy(ptr, argv[i], argsize);
- ptr += argsize;
+ ptr += argsize;
}
/* Terminate the argv[] list */
- bin->argv[i] = (FAR char *)NULL;
+ argvbuf[i] = NULL;
}
}
- return OK;
+ return (FAR char * const *)argvbuf;
#else
- /* Just save the caller's argv pointer */
+ /* Just return the caller's argv pointer */
- bin->argv = argv;
- return OK;
+ return argv;
#endif
}
@@ -158,7 +154,7 @@ int binfmt_copyargv(FAR struct binary_s *bin, FAR char * const *argv)
* Release the copied argv[] list.
*
* Input Parameters:
- * binp - Load structure
+ * argv - Argument list
*
* Returned Value:
* None
@@ -166,21 +162,16 @@ int binfmt_copyargv(FAR struct binary_s *bin, FAR char * const *argv)
****************************************************************************/
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
-void binfmt_freeargv(FAR struct binary_s *binp)
+void binfmt_freeargv(FAR char * const *argv)
{
/* Is there an allocated argument buffer */
- if (binp->argbuffer)
+ if (argv)
{
/* Free the argument buffer */
- kmm_free(binp->argbuffer);
+ kmm_free((FAR char **)argv);
}
-
- /* Nullify the allocated argv[] array and the argument buffer pointers */
-
- binp->argbuffer = (FAR char *)NULL;
- binp->argv = (FAR char **)NULL;
}
#endif
diff --git a/binfmt/binfmt_dumpmodule.c b/binfmt/binfmt_dumpmodule.c
index 6c7a78c..84db104 100644
--- a/binfmt/binfmt_dumpmodule.c
+++ b/binfmt/binfmt_dumpmodule.c
@@ -58,7 +58,6 @@ int binfmt_dumpmodule(FAR const struct binary_s *bin)
{
binfo("Module:\n");
binfo(" filename: %s\n", bin->filename);
- binfo(" argv: %p\n", bin->argv);
binfo(" entrypt: %p\n", bin->entrypt);
binfo(" mapped: %p size=%zd\n", bin->mapped, bin->mapsize);
binfo(" alloc: %p %p %p\n", bin->alloc[0],
diff --git a/binfmt/binfmt_exec.c b/binfmt/binfmt_exec.c
index e16ecb3..1d72634 100644
--- a/binfmt/binfmt_exec.c
+++ b/binfmt/binfmt_exec.c
@@ -92,22 +92,13 @@ int exec_spawn(FAR const char *filename, FAR char * const *argv,
bin->exports = exports;
bin->nexports = nexports;
- /* Copy the argv[] list */
-
- ret = binfmt_copyargv(bin, argv);
- if (ret < 0)
- {
- berr("ERROR: Failed to copy argv[]: %d\n", ret);
- goto errout_with_bin;
- }
-
/* Load the module into memory */
ret = load_module(bin);
if (ret < 0)
{
berr("ERROR: Failed to load program '%s': %d\n", filename, ret);
- goto errout_with_argv;
+ goto errout_with_bin;
}
/* Update the spawn attribute */
@@ -136,7 +127,7 @@ int exec_spawn(FAR const char *filename, FAR char * const *argv,
/* Then start the module */
- pid = exec_module(bin);
+ pid = exec_module(bin, argv);
if (pid < 0)
{
ret = pid;
@@ -159,7 +150,6 @@ int exec_spawn(FAR const char *filename, FAR char * const *argv,
#else
/* Free the binary_s structure here */
- binfmt_freeargv(bin);
kmm_free(bin);
/* TODO: How does the module get unloaded in this case? */
@@ -172,8 +162,6 @@ int exec_spawn(FAR const char *filename, FAR char * const *argv,
errout_with_lock:
sched_unlock();
unload_module(bin);
-errout_with_argv:
- binfmt_freeargv(bin);
errout_with_bin:
kmm_free(bin);
errout:
diff --git a/binfmt/binfmt_execmodule.c b/binfmt/binfmt_execmodule.c
index 3acfeec..5a8aeeb 100644
--- a/binfmt/binfmt_execmodule.c
+++ b/binfmt/binfmt_execmodule.c
@@ -111,7 +111,7 @@ static void exec_ctors(FAR void *arg)
*
****************************************************************************/
-int exec_module(FAR const struct binary_s *binp)
+int exec_module(FAR const struct binary_s *binp, FAR char * const *argv)
{
FAR struct task_tcb_s *tcb;
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
@@ -139,6 +139,16 @@ int exec_module(FAR const struct binary_s *binp)
return -ENOMEM;
}
+ if (argv)
+ {
+ argv = binfmt_copyargv(argv);
+ if (!argv)
+ {
+ ret = -ENOMEM;
+ goto errout_with_tcb;
+ }
+ }
+
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
/* Instantiate the address environment containing the user heap */
@@ -146,6 +156,7 @@ int exec_module(FAR const struct binary_s *binp)
if (ret < 0)
{
berr("ERROR: up_addrenv_select() failed: %d\n", ret);
+ binfmt_freeargv(argv);
goto errout_with_tcb;
}
#endif
@@ -157,21 +168,14 @@ int exec_module(FAR const struct binary_s *binp)
/* Initialize the task */
ret = nxtask_init(tcb, binp->filename, binp->priority,
- NULL, binp->stacksize, binp->entrypt, binp->argv);
+ NULL, binp->stacksize, binp->entrypt, argv);
+ binfmt_freeargv(argv);
if (ret < 0)
{
berr("nxtask_init() failed: %d\n", ret);
goto errout_with_addrenv;
}
- /* We can free the argument buffer now.
- * REVISIT: It is good to free up memory as soon as possible, but
- * unfortunately here 'binp' is 'const'. So to do this properly, we will
- * have to make some more extensive changes.
- */
-
- binfmt_freeargv((FAR struct binary_s *)binp);
-
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
/* Allocate the kernel stack */
@@ -264,9 +268,8 @@ errout_with_tcbinit:
errout_with_addrenv:
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
up_addrenv_restore(&oldenv);
-
-errout_with_tcb:
#endif
+errout_with_tcb:
kmm_free(tcb);
return ret;
}
diff --git a/binfmt/binfmt_unloadmodule.c b/binfmt/binfmt_unloadmodule.c
index 793e533..323e3c6 100644
--- a/binfmt/binfmt_unloadmodule.c
+++ b/binfmt/binfmt_unloadmodule.c
@@ -150,10 +150,6 @@ int unload_module(FAR struct binary_s *binp)
}
#endif
- /* Free any allocated argv[] strings */
-
- binfmt_freeargv(binp);
-
/* Unmap mapped address spaces */
if (binp->mapped)
diff --git a/include/nuttx/binfmt/binfmt.h b/include/nuttx/binfmt/binfmt.h
index 38dbf39..67fccb6 100644
--- a/include/nuttx/binfmt/binfmt.h
+++ b/include/nuttx/binfmt/binfmt.h
@@ -64,12 +64,6 @@ struct binary_s
/* Information provided to the loader to load and bind a module */
FAR const char *filename; /* Full path to the binary to be loaded (See NOTE 1 above) */
-#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
- FAR char *argbuffer; /* Allocated argument list */
- FAR char **argv; /* Copy of argument list */
-#else
- FAR char * const *argv; /* Argument list */
-#endif
FAR const struct symtab_s *exports; /* Table of exported symbols */
int nexports; /* The number of symbols in exports[] */
@@ -234,7 +228,7 @@ int unload_module(FAR struct binary_s *bin);
*
****************************************************************************/
-int exec_module(FAR const struct binary_s *bin);
+int exec_module(FAR const struct binary_s *binp, FAR char * const *argv);
/****************************************************************************
* Name: exec