Re: [PATCH v2 1/2] Clean stale environment pointer in finish_command()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 10, 2014 at 03:41:09PM +0100, Johannes Schindelin wrote:

> > However, start_command() will set "env" to env_array.argv only if "env"
> > was unset to begin with, and if it was already set, the caller will need
> > the original value. Therefore, we need to be very careful only to reset
> > "env" in finish_command() when it has been initialized in start_command().
> 
> In case it was unclear: this is needed for the the suggested switch from the
> previous method to construct the environment to the new env_array method
> to work.
> 
> (The env_array method unfortunately requires the code to initialize the
> environment twice because finish_command() insists on always releasing the
> env_array, even if the caller may want to reuse the generated array).

I don't think this is "unfortunately"; freeing the memory was the entire
purpose in adding env_array. If you want to easily reuse the same
environment in multiple commands, it is still perfectly fine to use
"env" directly, like:

  struct argv_array env = ARGV_ARRAY_INIT;
  struct child_process one = CHILD_PROCESS_INIT;
  struct child_process two = CHILD_PROCESS_INIT;

  ... setup env with argv_array_push ...

  one.argv = foo;
  one.env = env.argv;
  run_command(&one);

  two.argv = bar;
  two.env = env.argv;
  run_command(&two);

  argv_array_clear(&env);

You do not get the benefit of the auto-cleanup (you have to call
argv_array_clear yourself), but that is less bad than repeating the
setup of "env" twice.

It may be that Documentation/technical/api-run-command.txt needs to
make this more explicit (I just read over it and it looks OK to me. But
since I am the one who designed the feature, I am not the best test of
whether it is sufficiently clear).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]