Noux: store environment variables zero-separated

With this patch environment variables always get stored as zero-separated
strings in buffers of type 'Sysio::Env'. This fixes the problem that
environment variables with non-alphanumeric characters did not get set
correctly in child processes.

Fixes #340.

ports/src/lib/libc_noux/plugin.cc

@@ -242,7 +242,6 @@ static bool serialize_string_array(char const * const * array, char *dst, Genode
 			return false;
 
 		Genode::strncpy(dst, array[i], dst_len);
-
 		dst += curr_len;
 		dst_len -= curr_len;
 	}
@@ -1697,38 +1696,16 @@ void init_libc_noux(void)
 	Genode::Dataspace_capability env_ds = env_rom.dataspace();
 	char *env_string = (char *)Genode::env()->rm_session()->attach(env_ds);
 
-	enum { ENV_MAX_SIZE       = 4096,
+	enum { ENV_MAX_ENTRIES =  128 };
-	       ENV_MAX_ENTRIES    =  128,
-	       ENV_KEY_MAX_SIZE   =  256,
-	       ENV_VALUE_MAX_SIZE = 1024 };
-
-	static char  env_buf[ENV_MAX_SIZE];
 	static char *env_array[ENV_MAX_ENTRIES];
 
 	unsigned num_entries = 0;  /* index within 'env_array' */
-	Genode::size_t i     = 0;  /* index within 'env_buf' */
 
-	while ((num_entries < ENV_MAX_ENTRIES - 2) && (i < ENV_MAX_SIZE)) {
+	while (*env_string && (num_entries < ENV_MAX_ENTRIES - 1)) {
-
+		env_array[num_entries++] = env_string;
-		Genode::Arg arg = Genode::Arg_string::first_arg(env_string);
+		env_string += (strlen(env_string) + 1);
-		if (!arg.valid())
-			break;
-
-		char key_buf[ENV_KEY_MAX_SIZE],
-		     value_buf[ENV_VALUE_MAX_SIZE];
-
-		arg.key(key_buf, sizeof(key_buf));
-		arg.string(value_buf, sizeof(value_buf), "");
-
-		env_array[num_entries++] = env_buf + i;
-		Genode::snprintf(env_buf + i, ENV_MAX_SIZE - i,
-		                 "%s=%s", key_buf, value_buf);
-
-		i += Genode::strlen(env_buf + i) + 1;
-
-		/* remove processed arg from 'env_string' */
-		Genode::Arg_string::remove_arg(env_string, key_buf);
 	}
+	env_array[num_entries] = 0;
 
 	/* register list of environment variables at libc 'environ' pointer */
 	environ = env_array;

ports/src/noux/child.h

@@ -295,7 +295,7 @@ namespace Noux {
 			      Signal_receiver  *sig_rec,
 			      Dir_file_system  *root_dir,
 			      Args              const &args,
-			      char const       *env,
+			      Sysio::Env        const &env,
 			      char const       *pwd,
 			      Cap_session      *cap_session,
 			      Service_registry &parent_services,

ports/src/noux/child_env.h

@@ -39,43 +39,13 @@ namespace Noux {
 
 			char const *_binary_name;
 			char        _args[ARGS_SIZE + MAX_LEN_INTERPRETER_LINE];
-			char        _env[Sysio::ENV_MAX_LEN];
+			Sysio::Env  _env;
 
-			/**
-			 * Deserialize environment variable buffer into a
-			 * null-terminated string. The source env buffer contains a
-			 * list of strings separated by single 0 characters. Each
-			 * string has the form "name=value" (w/o the quotes). The end
-			 * of the list is marked by an additional 0 character. The
-			 * resulting string is a null-terminated string containing a
-			 * comma-separated list of environment variables.
-			 */
 			void _process_env(Sysio::Env env)
 			{
-				/**
+				memcpy(_env, env, sizeof(Sysio::Env));
-				 * In the following loop, 'i' is the index into the source
-				 * buffer, 'j' is the index into the destination buffer, 'env'
-				 * is the destination.
-				 */
-				for (unsigned i = 0, j = 0; i < Sysio::ENV_MAX_LEN && env[i]; )
-				{
-					char const *src = &env[i];
-
-					/* prepend a comma in front of each entry except for the first one */
-					if (i) {
-						snprintf(_env + j, sizeof(_env) - j, ",");
-						j++;
-					}
-
-					snprintf(_env + j, sizeof(_env) - j, "%s", src);
-
-					/* skip null separator in source string */
-					i += strlen(src) + 1;
-					j += strlen(src);
-				}
 			}
 
-
 			/**
 			 * Handle the case that the given binary needs an interpreter
 			 */
@@ -183,7 +153,7 @@ namespace Noux {
 
 			Args args() { return Args(_args, sizeof(_args)); }
 
-			char const *env() const { return _env; }
+			Sysio::Env const &env() const { return _env; }
 	};
 
 }

ports/src/noux/environment.h

@@ -31,9 +31,7 @@ namespace Noux {
 	{
 		private:
 
-			enum { ENV_DS_SIZE = 4096 };
+			Sysio::Env *_env;
-
-			char *_env;
 
 			Pwd::Path _pwd_path;
 
@@ -42,19 +40,19 @@ namespace Noux {
 			/**
 			 * \param env  comma-separated list of environment variables
 			 */
-			Environment(char const *env) :
+			Environment(Sysio::Env const &env) :
-				Attached_ram_dataspace(Genode::env()->ram_session(), ENV_DS_SIZE),
+				Attached_ram_dataspace(Genode::env()->ram_session(), sizeof(Sysio::Env)),
-				_env(local_addr<char>())
+				_env(local_addr<Sysio::Env>())
 			{
-				strncpy(_env, env, ENV_DS_SIZE);
+				memcpy(_env, env, sizeof(Sysio::Env));
 			}
 
 			using Attached_ram_dataspace::cap;
 
 			/**
-			 * Return list of environment variables as comma-separated list
+			 * Return list of environment variables as zero-separated list
 			 */
-			char const *env() { return _env; }
+			Sysio::Env const &env() { return *_env; }
 
 
 			/*******************
@@ -67,26 +65,6 @@ namespace Noux {
 			{
 				_pwd_path.import(pwd);
 				_pwd_path.remove_trailing('/');
-
-				char quoted[Sysio::MAX_PATH_LEN];
-				Range_checked_index<unsigned> i(0, sizeof(quoted));
-
-				try {
-					char const *s = _pwd_path.base();
-					quoted[i++] = '"';
-					while (*s) {
-						if (*s == '"')
-							quoted[i++] = '/';
-						quoted[i++] = *s++;
-					}
-					quoted[i++] = '"';
-					quoted[i] = 0;
-				} catch (Index_out_of_range) {
-					PERR("Could not set PWD, buffer too small");
-					return;
-				}
-
-				Arg_string::set_arg(_env, ENV_DS_SIZE, "PWD", quoted);
 				PINF("changed current work directory to %s", _pwd_path.base());
 			}
 	};

ports/src/noux/main.cc

@@ -20,6 +20,7 @@
 /* Noux includes */
 #include <child.h>
 #include <child_env.h>
+#include <noux_session/sysio.h>
 #include <vfs_io_channel.h>
 #include <terminal_io_channel.h>
 #include <dummy_input_io_channel.h>
@@ -674,42 +675,16 @@ static Noux::Args const &args_of_init_process()
 }
 
 
-static void quote(char *buf, Genode::size_t buf_len)
-{
-	/*
-	 * Make sure to leave space at the end of buffer for the finishing '"' and
-	 * the null-termination.
-	 */
-	char c = '"';
-	unsigned i = 0;
-	for (; c && (i + 2 < buf_len); i++)
-	{
-		/*
-		 * So shouldn't this have a special case for '"' characters inside the
-		 * string? This is actually not needed because such a string could
-		 * never be constructed via the XML config anyway. You can sneak in '"'
-		 * characters only by quoting them in the XML file. Then, however, they
-		 * are already quoted.
-		 */
-		char next_c = buf[i];
-		buf[i] = c;
-		c = next_c;
-	}
-	buf[i + 0] = '"';
-	buf[i + 1] = 0;
-}
-
-
 /**
  * Return string containing the environment variables of init
  *
- * The string is formatted according to the 'Genode::Arg_string' rules.
+ * The variable definitions are separated by zeros. The end of the string is
+ * marked with another zero.
  */
-static char const *env_string_of_init_process()
+static Noux::Sysio::Env &env_string_of_init_process()
 {
-	static char env_buf[4096];
+	static Noux::Sysio::Env env;
-
+	int index = 0;
-	Genode::Arg_string::set_arg(env_buf, sizeof(env_buf), "PWD", "\"/\"");
 
 	/* read environment variables for init process from config */
 	Genode::Xml_node start_node = Genode::config()->xml_node().sub_node("start");
@@ -720,15 +695,22 @@ static char const *env_string_of_init_process()
 
 			arg_node.attribute("name").value(name_buf, sizeof(name_buf));
 			arg_node.attribute("value").value(value_buf, sizeof(value_buf));
-			quote(value_buf, sizeof(value_buf));
 
-			Genode::Arg_string::set_arg(env_buf, sizeof(env_buf),
+			Genode::size_t env_var_size = Genode::strlen(name_buf) +
-			                            name_buf, value_buf);
+			                              Genode::strlen("=") +
+			                              Genode::strlen(value_buf) + 1;
+			if (index + env_var_size < sizeof(env)) {
+				Genode::snprintf(&env[index], env_var_size, "%s=%s", name_buf, value_buf);
+				index += env_var_size;
+			} else {
+				env[index] = 0;
+				break;
+			}
 		}
 	}
 	catch (Genode::Xml_node::Nonexistent_sub_node) { }
 
-	return env_buf;
+	return env;
 }