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

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

 



Hi,

On Tue, 11 Nov 2014, Junio C Hamano wrote:

> Jeff King <peff@xxxxxxxx> writes:
> 
> > 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.
> 
> Yeah, the above looks like the best option, better than using a
> single child_process and having to re-initialize fds and envs.

Okay, I have to say that I was led to believe that reusing the
child_process struct is okay because argv_array_clear() explicitly
reinitializes the env_array field, something that is useless churn unless
you plan to reuse the memory.

However, my personal taste says that reusing the same memory is more
elegant than to waste extra memory unnecessarily, so I will go with the
child_process_init() solution.

Ciao,
Dscho
--
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]