Yeqi Fu <fufuyqqq...@gmail.com> writes:
> Signed-off-by: Yeqi Fu <fufuyqqq...@gmail.com>
> ---
> include/qemu/envlist.h | 13 ++++++
> tests/unit/meson.build | 1 +
> tests/unit/test-envlist.c | 94 +++++++++++++++++++++++++++++++++++++++
> util/envlist.c | 71 ++++++++++++++++++++++++-----
> 4 files changed, 169 insertions(+), 10 deletions(-)
> create mode 100644 tests/unit/test-envlist.c
Thanks for adding the unit test.
>
> +/*
> + * Appends environment value to envlist. If the environment
> + * variable already exists, the new value is appended to the
> + * existing one.
> + *
> + * Returns 0 in success, errno otherwise.
> + */
> +int
> +envlist_appendenv(envlist_t *envlist, const char *env, const char *separator)
> +{
> + struct envlist_entry *entry = NULL;
> + const char *eq_sign;
> + size_t envname_len;
> +
> + if ((envlist == NULL) || (env == NULL) || (separator == NULL)) {
> + return -EINVAL;
> + }
> +
> + /* find out first equals sign in given env */
> + eq_sign = strchr(env, '=');
> + if (eq_sign == NULL) {
> + return -EINVAL;
> + }
> +
> + if (strchr(eq_sign + 1, '=') != NULL) {
> + return -EINVAL;
> + }
> +
> + envname_len = eq_sign - env + 1;
> +
> + /*
> + * If there already exists variable with given name,
> + * we append the new value to the existing one.
> + */
> + for (entry = envlist->el_entries.lh_first; entry != NULL;
> + entry = entry->ev_link.le_next) {
> + if (strncmp(entry->ev_var, env, envname_len) == 0) {
> + break;
> + }
> + }
> +
> + if (entry != NULL) {
> + char *new_env_value = NULL;
> + size_t new_env_len = strlen(entry->ev_var) + strlen(eq_sign)
> + + strlen(separator) + 1;
> + new_env_value = g_malloc(new_env_len);
> + strcpy(new_env_value, entry->ev_var);
> + strcat(new_env_value, separator);
> + strcat(new_env_value, eq_sign + 1);
> + g_free((char *)entry->ev_var);
> + entry->ev_var = new_env_value;
You could probably avoid messing about with strlens if you used the
handy glib functions:
if (entry != NULL) {
GString *new_val = g_string_new(entry->ev_var);
g_string_append(new_val, separator);
g_string_append(new_val, eq_sign + 1);
g_free(entry->ev_var);
entry->ev_var = g_string_free(new_val, false);
} else {
Also the fact you needed to cast entry->ev_var to g_free() should have
pointed out that now we can change env entries we need to drop the const
from the char *ev_var; definition in envlist_entry.
Otherwise with those fixes:
Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
