| Message ID | 20200924092314.1722645-43-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PULL,01/92] tests: add missing genh dependency | expand |
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
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 --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; +}
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(-)