Re: [Libguestfs] [PATCH nbdkit 8/9] eval, sh: Set $tmpdir before running the command, instead of globally.
On 4/15/20 11:16 AM, Richard W.M. Jones wrote:
The $tmpdir environment variable is used by the eval and sh plugins
to
communicate the path to the temporary directory created by the plugin
for shell scripts to use for temporary files.
Previously we set this environment variable globally (in .load()), but
this means the environment variable can be leaked into undesirable
places, eg. into the --run script:
$ nbdkit sh - --run 'echo tmpdir: $tmpdir' </dev/null
tmpdir: /tmp/nbdkitshco6MC9
Good catch.
By setting it only just before running the shell command it should
only be visible there. Note also we no longer use setenv(3), but
instead we manipulate environ.
After this change:
$ ./nbdkit sh - --run 'echo tmpdir: $tmpdir' </dev/null
tmpdir:
---
plugins/sh/call.h | 2 ++
plugins/eval/eval.c | 7 +------
plugins/sh/call.c | 11 +++++++++++
plugins/sh/sh.c | 7 +------
4 files changed, 15 insertions(+), 12 deletions(-)
As I asked elsewhere, can we arrange for main.c to call run_command()
prior to .get_ready, then sh and eval can still call setenv() during
.get_ready (at a point where it will not be picked up by the --run
process), more efficiently than copying the environ on every callback?
Or, even if we can't call setenv(), can we at least compute the
alternative environment up front...
+++ b/plugins/sh/call.c
@@ -106,6 +106,7 @@ call3 (const char *wbuf, size_t wbuflen, /* sent to stdin */
const char **argv) /* script + parameters */
{
const char *argv0 = argv[0]; /* script name, used in error messages */
+ CLEANUP_FREE_STRING_LIST char **env = NULL;
pid_t pid = -1;
int status;
int ret = ERROR;
@@ -122,6 +123,11 @@ call3 (const char *wbuf, size_t wbuflen, /* sent to stdin */
debug_call (argv);
+ /* Copy the environment, and add $tmpdir. */
+ env = copy_environ (environ, "tmpdir", tmpdir, NULL);
+ if (env == NULL)
+ goto error;
+
...and pass the precomputed environ to call3() rather than re-computing
it on every callback?
Can we add testsuite coverage for this?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org