Re: [PATCH v3 01/15] include: move qemu_*_exec_dir() to cutils

[Markus Armbruster <armbru@redhat.com>]

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The function is required by get_relocated_path() (already in cutils),
> and used by qemu-ga and may be generally useful.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/cutils.h                |   7 ++
>  include/qemu/osdep.h                 |   8 --
>  qemu-io.c                            |   1 +
>  storage-daemon/qemu-storage-daemon.c |   1 +
>  tests/qtest/fuzz/fuzz.c              |   1 +
>  util/cutils.c                        | 108 +++++++++++++++++++++++++++
>  util/oslib-posix.c                   |  81 --------------------
>  util/oslib-win32.c                   |  36 ---------
>  8 files changed, 118 insertions(+), 125 deletions(-)
>
> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> index 5c6572d444..40e10e19a7 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -193,6 +193,13 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n);
>   */
>  int qemu_pstrcmp0(const char **str1, const char **str2);
>  
> +/* Find program directory, and save it for later usage with
> + * qemu_get_exec_dir().
> + * Try OS specific API first, if not working, parse from argv0. */
> +void qemu_init_exec_dir(const char *argv0);
> +
> +/* Get the saved exec dir.  */
> +const char *qemu_get_exec_dir(void);
>  
>  /**
>   * get_relocated_path:
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 1c1e7eca98..67cc465416 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -557,14 +557,6 @@ void qemu_set_cloexec(int fd);
>   */
>  char *qemu_get_local_state_dir(void);
>  
> -/* Find program directory, and save it for later usage with
> - * qemu_get_exec_dir().
> - * Try OS specific API first, if not working, parse from argv0. */
> -void qemu_init_exec_dir(const char *argv0);
> -
> -/* Get the saved exec dir.  */
> -const char *qemu_get_exec_dir(void);
> -
>  /**
>   * qemu_getauxval:
>   * @type: the auxiliary vector key to lookup
> diff --git a/qemu-io.c b/qemu-io.c
> index d70d3dd4fd..2bd7bfb650 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -16,6 +16,7 @@
>  #endif
>  
>  #include "qemu/help-texts.h"
> +#include "qemu/cutils.h"
>  #include "qapi/error.h"
>  #include "qemu-io.h"
>  #include "qemu/error-report.h"
> diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
> index 9b8b17f52e..c104817cdd 100644
> --- a/storage-daemon/qemu-storage-daemon.c
> +++ b/storage-daemon/qemu-storage-daemon.c
> @@ -44,6 +44,7 @@
>  
>  #include "qemu/help-texts.h"
>  #include "qemu-version.h"
> +#include "qemu/cutils.h"
>  #include "qemu/config-file.h"
>  #include "qemu/error-report.h"
>  #include "qemu/help_option.h"
> diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
> index a7a5e14fa3..0ad4ba9e94 100644
> --- a/tests/qtest/fuzz/fuzz.c
> +++ b/tests/qtest/fuzz/fuzz.c
> @@ -15,6 +15,7 @@
>  
>  #include <wordexp.h>
>  
> +#include "qemu/cutils.h"
>  #include "qemu/datadir.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/qtest.h"
> diff --git a/util/cutils.c b/util/cutils.c
> index b2777210e7..6cc7cc8cde 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -931,6 +931,114 @@ static inline const char *next_component(const char *dir, int *p_len)
>      return dir;
>  }
>  
> +static const char *exec_dir;
> +
> +void qemu_init_exec_dir(const char *argv0)
> +{
> +#ifdef G_OS_WIN32
> +    char *p;
> +    char buf[MAX_PATH];
> +    DWORD len;
> +
> +    if (exec_dir) {
> +        return;
> +    }
> +
> +    len = GetModuleFileName(NULL, buf, sizeof(buf) - 1);
> +    if (len == 0) {
> +        return;
> +    }
> +
> +    buf[len] = 0;
> +    p = buf + len - 1;
> +    while (p != buf && *p != '\\') {
> +        p--;
> +    }
> +    *p = 0;
> +    if (access(buf, R_OK) == 0) {
> +        exec_dir = g_strdup(buf);
> +    } else {
> +        exec_dir = CONFIG_BINDIR;
> +    }
> +#else
> +    char *p = NULL;
> +    char buf[PATH_MAX];
> +
> +    if (exec_dir) {
> +        return;
> +    }
> +
> +#if defined(__linux__)
> +    {
> +        int len;
> +        len = readlink("/proc/self/exe", buf, sizeof(buf) - 1);
> +        if (len > 0) {
> +            buf[len] = 0;
> +            p = buf;
> +        }
> +    }
> +#elif defined(__FreeBSD__) \
> +      || (defined(__NetBSD__) && defined(KERN_PROC_PATHNAME))
> +    {
> +#if defined(__FreeBSD__)
> +        static int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1};
> +#else
> +        static int mib[4] = {CTL_KERN, KERN_PROC_ARGS, -1, KERN_PROC_PATHNAME};
> +#endif
> +        size_t len = sizeof(buf) - 1;
> +
> +        *buf = '\0';
> +        if (!sysctl(mib, ARRAY_SIZE(mib), buf, &len, NULL, 0) &&
> +            *buf) {
> +            buf[sizeof(buf) - 1] = '\0';
> +            p = buf;
> +        }
> +    }
> +#elif defined(__APPLE__)
> +    {
> +        char fpath[PATH_MAX];
> +        uint32_t len = sizeof(fpath);
> +        if (_NSGetExecutablePath(fpath, &len) == 0) {
> +            p = realpath(fpath, buf);
> +            if (!p) {
> +                return;
> +            }
> +        }
> +    }
> +#elif defined(__HAIKU__)
> +    {
> +        image_info ii;
> +        int32_t c = 0;
> +
> +        *buf = '\0';
> +        while (get_next_image_info(0, &c, &ii) == B_OK) {
> +            if (ii.type == B_APP_IMAGE) {
> +                strncpy(buf, ii.name, sizeof(buf));
> +                buf[sizeof(buf) - 1] = 0;
> +                p = buf;
> +                break;
> +            }
> +        }
> +    }
> +#endif
> +    /* If we don't have any way of figuring out the actual executable
> +       location then try argv[0].  */
> +    if (!p && argv0) {
> +        p = realpath(argv0, buf);
> +    }
> +    if (p) {
> +        exec_dir = g_path_get_dirname(p);
> +    } else {
> +        exec_dir = CONFIG_BINDIR;
> +    }
> +#endif
> +}

Combines code moved from oslib-posix.c and oslib-win32.c with #if.
Sounds like a step backward, until you realize just how many #if there
already are.  Okay.

> +
> +const char *qemu_get_exec_dir(void)
> +{
> +    return exec_dir;
> +}
> +
>  char *get_relocated_path(const char *dir)
>  {
>      size_t prefix_len = strlen(CONFIG_PREFIX);

Keeping qemu_init_exec_dir() and qemu_get_exec_dir() next to
get_relocated_path() makes sense.  However, util/cutils.c is getting
rather long: almost 700 SLOC, and I feel the new code stretches its
"Simple C functions to supplement the C library" mandate.  Most of the
code there is string stuff.  Would you mind putting the file name stuff
into its own file?

Regardless,
Reviewed-by: Markus Armbruster <armbru@redhat.com>

[...]