From 78b0bd57f707851f17d4fa1c347e0f3a983d1057 Mon Sep 17 00:00:00 2001 From: Christian Prochaska Date: Tue, 28 Aug 2012 17:24:59 +0200 Subject: [PATCH] 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 | 33 +++----------------- ports/src/noux/child.h | 2 +- ports/src/noux/child_env.h | 36 ++------------------- ports/src/noux/environment.h | 36 +++++---------------- ports/src/noux/main.cc | 52 ++++++++++--------------------- 5 files changed, 33 insertions(+), 126 deletions(-) diff --git a/ports/src/lib/libc_noux/plugin.cc b/ports/src/lib/libc_noux/plugin.cc index feaeb8d24..023f3d203 100644 --- a/ports/src/lib/libc_noux/plugin.cc +++ b/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, - ENV_MAX_ENTRIES = 128, - ENV_KEY_MAX_SIZE = 256, - ENV_VALUE_MAX_SIZE = 1024 }; - - static char env_buf[ENV_MAX_SIZE]; + enum { ENV_MAX_ENTRIES = 128 }; 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)) { - - Genode::Arg arg = Genode::Arg_string::first_arg(env_string); - 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); + while (*env_string && (num_entries < ENV_MAX_ENTRIES - 1)) { + env_array[num_entries++] = env_string; + env_string += (strlen(env_string) + 1); } + env_array[num_entries] = 0; /* register list of environment variables at libc 'environ' pointer */ environ = env_array; diff --git a/ports/src/noux/child.h b/ports/src/noux/child.h index aefac5a1b..eb7385b70 100644 --- a/ports/src/noux/child.h +++ b/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, diff --git a/ports/src/noux/child_env.h b/ports/src/noux/child_env.h index a32e258c3..a144dacf5 100644 --- a/ports/src/noux/child_env.h +++ b/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) { - /** - * 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); - } + memcpy(_env, env, sizeof(Sysio::Env)); } - /** * 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; } }; } diff --git a/ports/src/noux/environment.h b/ports/src/noux/environment.h index 97c00e8bc..7c06b984f 100644 --- a/ports/src/noux/environment.h +++ b/ports/src/noux/environment.h @@ -31,9 +31,7 @@ namespace Noux { { private: - enum { ENV_DS_SIZE = 4096 }; - - char *_env; + Sysio::Env *_env; Pwd::Path _pwd_path; @@ -42,19 +40,19 @@ namespace Noux { /** * \param env comma-separated list of environment variables */ - Environment(char const *env) : - Attached_ram_dataspace(Genode::env()->ram_session(), ENV_DS_SIZE), - _env(local_addr()) + Environment(Sysio::Env const &env) : + Attached_ram_dataspace(Genode::env()->ram_session(), sizeof(Sysio::Env)), + _env(local_addr()) { - 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 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()); } }; diff --git a/ports/src/noux/main.cc b/ports/src/noux/main.cc index 552270fdb..3d62fe5fc 100644 --- a/ports/src/noux/main.cc +++ b/ports/src/noux/main.cc @@ -20,6 +20,7 @@ /* Noux includes */ #include #include +#include #include #include #include @@ -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]; - - Genode::Arg_string::set_arg(env_buf, sizeof(env_buf), "PWD", "\"/\""); + static Noux::Sysio::Env env; + int index = 0; /* 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), - name_buf, value_buf); + Genode::size_t env_var_size = Genode::strlen(name_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; }