[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 42/92] cutils: introduce get_relocated_path
From: |
Peter Maydell |
Subject: |
Re: [PULL 42/92] cutils: introduce get_relocated_path |
Date: |
Mon, 2 Nov 2020 18:05:25 +0000 |
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,
Peter Maydell <=