diff --git a/binfmt/binfmt.h b/binfmt/binfmt.h index 011e7c3ca4..2d4b7d92f2 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 af429d54c9..352f254b69 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 6c7a78c4de..84db10440c 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 e16ecb3eeb..1d72634868 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 3acfeecc2a..5a8aeebefc 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 793e533e00..323e3c6280 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 38dbf39059..67fccb6755 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