[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 01/15] include: move qemu_*_exec_dir() to cutils
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v3 01/15] include: move qemu_*_exec_dir() to cutils |
Date: |
Mon, 23 May 2022 14:23:44 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
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>
[...]