Re: [PATCH v6 3/7] Remove prefixes from path configuration macros

[Paolo Bonzini <pbonzini@redhat.com>]

On 6/15/22 17:56, Akihiko Odaki wrote:
> The path configuration macros are often supplied to
> get_relocated_path(), and the function had some logics to remove the
> prefixes.
> 
> With this change, the prefixes are removed from those macros and
> get_relocated_path() is also simplified.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>

This doesn't work if somebody configures with --prefix=/usr 
--libexecdir=/usr/libexec.  Adding the prefixes in meson.build was done 
as a canonicalization step so that the C code has less cases to care about.

Paolo

> ---
>   include/qemu/cutils.h |  2 +-
>   meson.build           | 21 ++++++++++-----------
>   qemu-options.hx       | 11 +++++------
>   util/cutils.c         | 34 +++++++---------------------------
>   4 files changed, 23 insertions(+), 45 deletions(-)
> 
> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> index 40e10e19a7e..57de1da5c95 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -207,7 +207,7 @@ const char *qemu_get_exec_dir(void);
>    *
>    * Returns a path for @dir that uses the directory of the running executable
>    * as the prefix.  For example, if `bindir` is `/usr/bin` and @dir is
> - * `/usr/share/qemu`, the function will append `../share/qemu` to the
> + * `share/qemu`, the function will append `../share/qemu` to the
>    * directory that contains the running executable and return the result.
>    * The returned string should be freed by the caller.
>    */
> diff --git a/meson.build b/meson.build
> index 0c2e11ff071..01d5e32615e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1679,18 +1679,17 @@ config_host_data.set_quoted('CONFIG_TLS_PRIORITY', get_option('tls_priority'))
>   if iasl.found()
>     config_host_data.set_quoted('CONFIG_IASL', iasl.full_path())
>   endif
> -config_host_data.set_quoted('CONFIG_BINDIR', get_option('prefix') / get_option('bindir'))
> +config_host_data.set_quoted('CONFIG_BINDIR', get_option('bindir'))
>   config_host_data.set_quoted('CONFIG_PREFIX', get_option('prefix'))
> -config_host_data.set_quoted('CONFIG_QEMU_CONFDIR', get_option('prefix') / qemu_confdir)
> -config_host_data.set_quoted('CONFIG_QEMU_DATADIR', get_option('prefix') / qemu_datadir)
> -config_host_data.set_quoted('CONFIG_QEMU_DESKTOPDIR', get_option('prefix') / qemu_desktopdir)
> -config_host_data.set_quoted('CONFIG_QEMU_FIRMWAREPATH', get_option('prefix') / get_option('qemu_firmwarepath'))
> -config_host_data.set_quoted('CONFIG_QEMU_HELPERDIR', get_option('prefix') / get_option('libexecdir'))
> -config_host_data.set_quoted('CONFIG_QEMU_ICONDIR', get_option('prefix') / qemu_icondir)
> -config_host_data.set_quoted('CONFIG_QEMU_LOCALEDIR', get_option('prefix') / get_option('localedir'))
> -config_host_data.set_quoted('CONFIG_QEMU_LOCALSTATEDIR', get_option('prefix') / get_option('localstatedir'))
> -config_host_data.set_quoted('CONFIG_QEMU_MODDIR', get_option('prefix') / qemu_moddir)
> -config_host_data.set_quoted('CONFIG_SYSCONFDIR', get_option('prefix') / get_option('sysconfdir'))
> +config_host_data.set_quoted('CONFIG_QEMU_CONFDIR', qemu_confdir)
> +config_host_data.set_quoted('CONFIG_QEMU_DATADIR', qemu_datadir)
> +config_host_data.set_quoted('CONFIG_QEMU_FIRMWAREPATH', get_option('qemu_firmwarepath'))
> +config_host_data.set_quoted('CONFIG_QEMU_HELPERDIR', get_option('libexecdir'))
> +config_host_data.set_quoted('CONFIG_QEMU_ICONDIR', qemu_icondir)
> +config_host_data.set_quoted('CONFIG_QEMU_LOCALEDIR', get_option('localedir'))
> +config_host_data.set_quoted('CONFIG_QEMU_LOCALSTATEDIR', get_option('localstatedir'))
> +config_host_data.set_quoted('CONFIG_QEMU_MODDIR', qemu_moddir)
> +config_host_data.set_quoted('CONFIG_SYSCONFDIR', get_option('sysconfdir'))
>   
>   if config_host.has_key('CONFIG_MODULES')
>     config_host_data.set('CONFIG_STAMP', run_command(
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 377d22fbd82..f0ae8f44ff2 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2661,12 +2661,11 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>       "         [,poll-us=n]\n"
>       "                configure a host TAP network backend with ID 'str'\n"
>       "                connected to a bridge (default=" DEFAULT_BRIDGE_INTERFACE ")\n"
> -    "                use network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
> -    "                to configure it and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
> -    "                to deconfigure it\n"
> +    "                use custom network script 'file' to configure it (optional)\n"
> +    "                use custom network script 'dfile' to deconfigure it (optional)\n"
>       "                use '[down]script=no' to disable script execution\n"
> -    "                use network helper 'helper' (default=" DEFAULT_BRIDGE_HELPER ") to\n"
> -    "                configure it\n"
> +    "                use custom network helper 'helper' to\n"
> +    "                configure it (optional)\n"
>       "                use 'fd=h' to connect to an already opened TAP interface\n"
>       "                use 'fds=x:y:...:z' to connect to already opened multiqueue capable TAP interfaces\n"
>       "                use 'sndbuf=nbytes' to limit the size of the send buffer (the\n"
> @@ -2684,7 +2683,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>       "-netdev bridge,id=str[,br=bridge][,helper=helper]\n"
>       "                configure a host TAP network backend with ID 'str' that is\n"
>       "                connected to a bridge (default=" DEFAULT_BRIDGE_INTERFACE ")\n"
> -    "                using the program 'helper (default=" DEFAULT_BRIDGE_HELPER ")\n"
> +    "                using the custom program 'helper' (optional)\n"
>   #endif
>   #ifdef __linux__
>       "-netdev l2tpv3,id=str,src=srcaddr,dst=dstaddr[,srcport=srcport][,dstport=dstport]\n"
> diff --git a/util/cutils.c b/util/cutils.c
> index a58bcfd80e7..983db97b4df 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -917,13 +917,6 @@ int qemu_pstrcmp0(const char **str1, const char **str2)
>       return g_strcmp0(*str1, *str2);
>   }
>   
> -static inline bool starts_with_prefix(const char *dir)
> -{
> -    size_t prefix_len = strlen(CONFIG_PREFIX);
> -    return !memcmp(dir, CONFIG_PREFIX, prefix_len) &&
> -        (!dir[prefix_len] || G_IS_DIR_SEPARATOR(dir[prefix_len]));
> -}
> -
>   /* Return the next path component in dir, and store its length in *p_len.  */
>   static inline const char *next_component(const char *dir, int *p_len)
>   {
> @@ -967,7 +960,7 @@ void qemu_init_exec_dir(const char *argv0)
>       if (access(buf, R_OK) == 0) {
>           exec_dir = g_strdup(buf);
>       } else {
> -        exec_dir = CONFIG_BINDIR;
> +        exec_dir = CONFIG_PREFIX G_DIR_SEPARATOR_S CONFIG_BINDIR;
>       }
>   #else
>       char *p = NULL;
> @@ -1038,7 +1031,7 @@ void qemu_init_exec_dir(const char *argv0)
>       if (p) {
>           exec_dir = g_path_get_dirname(p);
>       } else {
> -        exec_dir = CONFIG_BINDIR;
> +        exec_dir = CONFIG_PREFIX G_DIR_SEPARATOR_S CONFIG_BINDIR;
>       }
>   #endif
>   }
> @@ -1050,39 +1043,26 @@ const char *qemu_get_exec_dir(void)
>   
>   char *get_relocated_path(const char *dir)
>   {
> -    size_t prefix_len = strlen(CONFIG_PREFIX);
>       const char *bindir = CONFIG_BINDIR;
>       const char *exec_dir = qemu_get_exec_dir();
>       GString *result;
> -    int len_dir, len_bindir;
> +    int len_bindir;
>   
>       /* Fail if qemu_init_exec_dir was not called.  */
>       assert(exec_dir[0]);
> -    if (!starts_with_prefix(dir) || !starts_with_prefix(bindir)) {
> -        return g_strdup(dir);
> -    }
>   
>       result = g_string_new(exec_dir);
>   
> -    /* Advance over common components.  */
> -    len_dir = len_bindir = prefix_len;
> -    do {
> -        dir += len_dir;
> -        bindir += len_bindir;
> -        dir = next_component(dir, &len_dir);
> -        bindir = next_component(bindir, &len_bindir);
> -    } while (len_dir && len_dir == len_bindir && !memcmp(dir, bindir, len_dir));
> -
>       /* Ascend from bindir to the common prefix with dir.  */
> +    len_bindir = 0;
>       while (len_bindir) {
>           bindir += len_bindir;
>           g_string_append(result, "/..");
>           bindir = next_component(bindir, &len_bindir);
>       }
>   
> -    if (*dir) {
> -        assert(G_IS_DIR_SEPARATOR(dir[-1]));
> -        g_string_append(result, dir - 1);
> -    }
> +    g_string_append_c(result, G_DIR_SEPARATOR);
> +    g_string_append(result, dir);
> +
>       return g_string_free(result, false);
>   }