Re: [RFC v2 6/6] linux-user: Add '-native-bypass' option

["Alex Bennée" <alex.bennee@linaro.org>]

Yeqi Fu <fufuyqqqqqq@gmail.com> writes:

> Signed-off-by: Yeqi Fu <fufuyqqqqqq@gmail.com>
> ---
>  include/qemu/envlist.h |  1 +
>  linux-user/main.c      | 23 +++++++++++++++++
>  util/envlist.c         | 56 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 80 insertions(+)
>
> diff --git a/include/qemu/envlist.h b/include/qemu/envlist.h
> index 6006dfae44..865eb18e17 100644
> --- a/include/qemu/envlist.h
> +++ b/include/qemu/envlist.h
> @@ -7,6 +7,7 @@ envlist_t *envlist_create(void);
>  void envlist_free(envlist_t *);
>  int envlist_setenv(envlist_t *, const char *);
>  int envlist_unsetenv(envlist_t *, const char *);
> +int envlist_appendenv(envlist_t *, const char *, const char *);
>  int envlist_parse_set(envlist_t *, const char *);
>  int envlist_parse_unset(envlist_t *, const char *);
>  char **envlist_to_environ(const envlist_t *, size_t *);
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 5e6b2e1714..313c116b3b 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -125,6 +125,8 @@ static void usage(int exitcode);
>  static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
>  const char *qemu_uname_release;
>  
> +static const char *native_lib;
> +
>  #if !defined(TARGET_DEFAULT_STACK_SIZE)
>  /* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
>     we allocate a bigger stack. Need a better solution, for example
> @@ -293,6 +295,13 @@ static void handle_arg_set_env(const char *arg)
>      free(r);
>  }
>  
> +#if defined(CONFIG_USER_ONLY)  && defined(CONFIG_USER_NATIVE_CALL)

CONFIG_USER_ONLY is a pointless check as by definition this file is
user-mode only.

> +static void handle_arg_native_bypass(const char *arg)
> +{
> +    native_lib = arg;

canonicalise the path and maybe verify it exists/is accessible?

> +}
> +#endif
> +
>  static void handle_arg_unset_env(const char *arg)
>  {
>      char *r, *p, *token;
> @@ -522,6 +531,10 @@ static const struct qemu_argument arg_table[] = {
>       "",           "Generate a /tmp/perf-${pid}.map file for perf"},
>      {"jitdump",    "QEMU_JITDUMP",     false, handle_arg_jitdump,
>       "",           "Generate a jit-${pid}.dump file for perf"},
> +#if defined(CONFIG_USER_ONLY)  && defined(CONFIG_USER_NATIVE_CALL)

see above re CONFIG_USER_ONLY.

> +    {"native-bypass", "QEMU_NATIVE_BYPASS", true, handle_arg_native_bypass,
> +     "",           "native bypass for library calls in user mode only."},
> +#endif
>      {NULL, NULL, false, NULL, NULL, NULL}
>  };
>  
> @@ -826,6 +839,16 @@ int main(int argc, char **argv, char **envp)
>          }
>      }
>  
> +    /* Set the library for native bypass  */
> +    if (native_lib != NULL) {
> +        char *token = malloc(strlen(native_lib) + 12);
> +        strcpy(token, "LD_PRELOAD=");
> +        strcat(token, native_lib);
> +         if (envlist_appendenv(envlist, token, ":") != 0) {
> +            usage(EXIT_FAILURE);
> +        }
> +    }
> +
>      target_environ = envlist_to_environ(envlist, NULL);
>      envlist_free(envlist);
>  
> diff --git a/util/envlist.c b/util/envlist.c
> index db937c0427..713d52497e 100644
> --- a/util/envlist.c
> +++ b/util/envlist.c
> @@ -201,6 +201,62 @@ envlist_unsetenv(envlist_t *envlist, const char *env)
>      return (0);
>  }
>

I think adding this function should be a separate commit. It would be
nice to add some unittests for the functionality while we are at it.

> +/*
> + * 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)) {
> +        return (EINVAL);
> +    }
> +
> +    /* find out first equals sign in given env */
> +    eq_sign = strchr(env, '=');
> +    if (eq_sign == 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);

See other comments about string functions.

> +        g_free((char *)entry->ev_var);
> +        entry->ev_var = new_env_value;
> +    } else {
> +        envlist->el_count++;
> +        entry = g_malloc(sizeof(*entry));
> +        entry->ev_var = g_strdup(env);
> +        QLIST_INSERT_HEAD(&envlist->el_entries, entry, ev_link);
> +    }
> +
> +    return (0);
> +}
> +
>  /*
>   * Returns given envlist as array of strings (in same form that
>   * global variable environ is).  Caller must free returned memory


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro