[PULL 42/92] cutils: introduce get_relocated_path

Paolo Bonzini posted 92 patches 5 years, 4 months ago
Maintainers: Fam Zheng <fam@euphon.net>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Markus Armbruster <armbru@redhat.com>, Bandan Das <bsd@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>, Michael Roth <mdroth@linux.vnet.ibm.com>, Peter Maydell <peter.maydell@linaro.org>, Jason Wang <jasowang@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Sergio Lopez <slp@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Hannes Reinecke <hare@suse.com>, Stefan Weil <sw@weilnetz.de>, Cleber Rosa <crosa@redhat.com>, Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>, David Hildenbrand <david@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Eric Blake <eblake@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Alexander Bulekov <alxndr@bu.edu>, "Michael S. Tsirkin" <mst@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Max Reitz <mreitz@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Sunil Muthuswamy <sunilmut@microsoft.com>, Richard Henderson <rth@twiddle.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Kevin Wolf <kwolf@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Halil Pasic <pasic@linux.ibm.com>
There is a newer version of this series
[PULL 42/92] cutils: introduce get_relocated_path
Posted by Paolo Bonzini 5 years, 4 months ago
Add the function that will compute a relocated version of the
directories in CONFIG_QEMU_*DIR and CONFIG_QEMU_*PATH.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/cutils.h | 12 +++++++++
 meson.build           |  4 +--
 util/cutils.c         | 61 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index eb59852dfd..3a86ec0321 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -184,4 +184,16 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n);
  */
 int qemu_pstrcmp0(const char **str1, const char **str2);
 
+
+/**
+ * get_relocated_path:
+ * @dir: the directory (typically a `CONFIG_*DIR` variable) to be relocated.
+ *
+ * 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
+ * directory that contains the running executable and return the result.
+ */
+char *get_relocated_path(const char *dir);
+
 #endif
diff --git a/meson.build b/meson.build
index 9a4ade7f98..ace15dc8c0 100644
--- a/meson.build
+++ b/meson.build
@@ -560,9 +560,9 @@ config_host_data.set('QEMU_VERSION_MINOR', meson.project_version().split('.')[1]
 config_host_data.set('QEMU_VERSION_MICRO', meson.project_version().split('.')[2])
 
 arrays = ['CONFIG_AUDIO_DRIVERS', 'CONFIG_BDRV_RW_WHITELIST', 'CONFIG_BDRV_RO_WHITELIST']
-strings = ['HOST_DSOSUF', 'CONFIG_IASL', 'bindir', 'qemu_confdir', 'qemu_datadir',
+strings = ['HOST_DSOSUF', 'CONFIG_IASL', 'bindir', 'prefix', 'qemu_confdir', 'qemu_datadir',
            'qemu_moddir', 'qemu_localstatedir', 'qemu_helperdir', 'qemu_localedir',
-           'qemu_icondir', 'qemu_desktopdir', 'qemu_firmwarepath']
+           'qemu_icondir', 'qemu_desktopdir', 'qemu_firmwarepath', 'sysconfdir']
 foreach k, v: config_host
   if arrays.contains(k)
     if v != ''
diff --git a/util/cutils.c b/util/cutils.c
index 36ce712271..8da34e04b0 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -889,3 +889,64 @@ 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)
+{
+    int len;
+    while (*dir && G_IS_DIR_SEPARATOR(*dir)) {
+        dir++;
+    }
+    len = 0;
+    while (dir[len] && !G_IS_DIR_SEPARATOR(dir[len])) {
+        len++;
+    }
+    *p_len = len;
+    return dir;
+}
+
+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;
+
+    /* Fail if qemu_init_exec_dir was not called.  */
+    assert(exec_dir[0]);
+    if (!starts_with_prefix(dir) || !starts_with_prefix(bindir)) {
+        return 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_bindir && !memcmp(dir, bindir, len_dir));
+
+    /* Ascend from bindir to the common prefix with dir.  */
+    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);
+    }
+    return result->str;
+}
-- 
2.26.2



Re: [PULL 42/92] cutils: introduce get_relocated_path
Posted by Peter Maydell 5 years, 3 months ago
On Thu, 24 Sep 2020 at 10:48, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Add the function that will compute a relocated version of the
> directories in CONFIG_QEMU_*DIR and CONFIG_QEMU_*PATH.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Hi; Coverity (CID 1432882) points out a bug in this code:

>  include/qemu/cutils.h | 12 +++++++++
>  meson.build           |  4 +--
>  util/cutils.c         | 61 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 75 insertions(+), 2 deletions(-)
>
> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> index eb59852dfd..3a86ec0321 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -184,4 +184,16 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n);
>   */
>  int qemu_pstrcmp0(const char **str1, const char **str2);
>
> +
> +/**
> + * get_relocated_path:
> + * @dir: the directory (typically a `CONFIG_*DIR` variable) to be relocated.
> + *
> + * 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
> + * directory that contains the running executable and return the result.
> + */
> +char *get_relocated_path(const char *dir);

Side note -- this function makes it the caller's responsibility
to free the string it returns, but it doesn't mention that in
this documentation comment.

> +

> +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;
> +
> +    /* Fail if qemu_init_exec_dir was not called.  */
> +    assert(exec_dir[0]);
> +    if (!starts_with_prefix(dir) || !starts_with_prefix(bindir)) {
> +        return strdup(dir);

Here we return memory allocated by strdup(), which must be
freed with free()...

> +    }
> +
> +    result = g_string_new(exec_dir);

...but here we allocate and will return a string that must
be freed with g_free(), leaving our caller stuck for how
to tell the difference.

Using g_strdup() instead of strdup() is the easy fix.

thanks
-- PMM

Re: [PULL 42/92] cutils: introduce get_relocated_path
Posted by Peter Maydell 5 years, 3 months ago
On Mon, 2 Nov 2020 at 18:05, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 24 Sep 2020 at 10:48, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > Add the function that will compute a relocated version of the
> > directories in CONFIG_QEMU_*DIR and CONFIG_QEMU_*PATH.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Hi; Coverity (CID 1432882)

Also 1432863 1432865 1432867 1432868 1432870 1432872 1432873
1432877 1432881, as it has helpfully filed a separate issue
for each callsite :-)

thanks
-- PMM