[Libguestfs] [PATCH nbdkit 8/9] eval, sh: Set $tmpdir before running the command, instead of globally.

[Eric Blake]

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