diff mbox series

[PULL,42/92] cutils: introduce get_relocated_path

Message ID 20200924092314.1722645-43-pbonzini@redhat.com
State Superseded
Headers show
Series Misc patches for 2020-09-24 | expand

Commit Message

Paolo Bonzini Sept. 24, 2020, 9:22 a.m. UTC
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(-)

Comments

Peter Maydell Nov. 2, 2020, 6:05 p.m. UTC | #1
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
Peter Maydell Nov. 2, 2020, 6:09 p.m. UTC | #2
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
diff mbox series

Patch

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;
+}