On 04/10, Jonathan Nieder wrote:

Brandon Williams wrote:

quoted

In order to avoid allocation between 'fork()' and 'exec()' prepare the
environment to be used in the child process prior to forking.

If using something like posix_spawn(), this would be needed anyway, so
I'll review it.

[...]

quoted

+++ b/run-command.c

[...]

quoted

+static char **prep_childenv(const char *const *deltaenv)
+{
+	char **childenv;
+	int childenv_nr = 0, childenv_alloc = 0;
+	int i;
+
+	for (i = 0; environ[i]; i++)
+		childenv_nr++;
+	for (i = 0; deltaenv && deltaenv[i]; i++)
+		childenv_alloc++;
+	/* Add one for the NULL termination */
+	childenv_alloc += childenv_nr + 1;
+
+	childenv = xcalloc(childenv_alloc, sizeof(char *));
+	memcpy(childenv, environ, childenv_nr * sizeof(char *));
+
+	/* merge in deltaenv */
+	for (i = 0; deltaenv && deltaenv[i]; i++)
+		childenv_nr = do_putenv(childenv, childenv_nr, deltaenv[i]);
+
+	return childenv;
+}

This potentially copies around most of 'environ' several times as it
adjusts for each deltaenv item. Can it be simplified? E.g.

The only time it copies anything is that first memcpy which makes a
duplicate of the environ, and then when an item is being removed in
do_putenv it will move the back of the array to fill in for the entry
being removed.

	struct argv_array result = ARGV_ARRAY_INIT;
	struct string_list mods = STRING_LIST_INIT_DUP;
	struct strbuf key = STRBUF_INIT;
	const char **p;

	for (p = cmd_env; *p; p++) {
		const char *equals = strchr(*p, '=');
		if (equals) {
			strbuf_reset(&key);
			strbuf_add(&key, *p, equals - *p);
			string_list_append(&mods, key.buf)->util = *p;
		} else {
			string_list_append(&mods, *p);
		}
	}
	string_list_sort(&mods);

	for (p = environ; *p; p++) {
		struct string_list_item *item;
		const char *equals = strchr(*p, '=');
		if (!equals)
			continue;
		strbuf_reset(&key);
		strbuf_add(&key, *p, equals - *p);
		item = string_list_lookup(&mods, key.buf);

		if (!item) /* no change */
			argv_array_push(&result, *p);
		else if (!item->util) /* unsetenv */
			; /* skip */
		else /* setenv */
			argv_array_push(&result, item->util);
	}

	strbuf_release(&key);
	string_list_clear(&mods);
	return argv_array_detach(&result);

This is probably still incomplete as I don't see how this accounts for
entries in 'cmd_env' which are being added to the environment and not
just replacing existing ones.

-- 
Brandon Williams