On Mon, May 03, 2021 at 12:01:43PM +0200, Tim Wiederhake wrote:
> When other preloaded libraries wrap and / or make calls to `realpath`
> (e.g. LLVM's AddessSanitizer), the second parameter is no longer
> guaranteed to be NULL.
>
> Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx>
> ---
> build-aux/syntax-check.mk | 2 +-
> tests/virfilemock.c | 12 ++++++------
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
> index 552d639119..03b7599abc 100644
> --- a/build-aux/syntax-check.mk
> +++ b/build-aux/syntax-check.mk
> @@ -1740,7 +1740,7 @@ exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics = \
> exclude_file_name_regexp--sc_po_check = ^(docs/|src/rpc/gendispatch\.pl$$|tests/commandtest.c$$)
>
> exclude_file_name_regexp--sc_prohibit_PATH_MAX = \
> - ^build-aux/syntax-check\.mk$$
> + ^(build-aux/syntax-check\.mk|tests/virfilemock.c)$$
>
> exclude_file_name_regexp--sc_prohibit_access_xok = \
> ^(src/util/virutil\.c)$$
> diff --git a/tests/virfilemock.c b/tests/virfilemock.c
> index 7c9174bdd9..cba66feac0 100644
> --- a/tests/virfilemock.c
> +++ b/tests/virfilemock.c
> @@ -24,7 +24,6 @@
> #if WITH_LINUX_MAGIC_H
> # include <linux/magic.h>
> #endif
> -#include <assert.h>
>
> #include "virmock.h"
> #include "virstring.h"
> @@ -186,15 +185,16 @@ realpath(const char *path, char *resolved)
>
> if (getenv("LIBVIRT_MTAB")) {
> const char *p;
> - char *ret;
>
> - assert(resolved == NULL);
> + if (!resolved)
> + resolved = g_new0(char, PATH_MAX);
> +
> if ((p = STRSKIP(path, "/some/symlink")))
> - ret = g_strdup_printf("/gluster%s", p);
> + g_snprintf(resolved, PATH_MAX, "/gluster%s", p);
> else
> - ret = g_strdup(path);
> + g_strlcpy(resolved, path, PATH_MAX);
>
> - return ret;
> + return resolved;
This seems way too complicated. It will be used only in tests where we
know what paths we use. Also the manpage for realpath states the
following:
If resolved_path is specified as NULL, then realpath() uses malloc(3) to
allocate a buffer of up to PATH_MAX bytes to hold the resolved pathname,
and returns a pointer to this buffer. The caller should deallocate this
buffer using free(3).
where the important word part is "up to PATH_MAX".
The code introduced by this patch always allocates PATH_MAX bytes.
How about this which to me looks like a good enough solution for tests:
if (getenv("LIBVIRT_MTAB")) {
const char *p;
if ((p = STRSKIP(path, "/some/symlink")))
resolved = g_strdup_printf("/gluster%s", p);
else
resolved = g_strdup(path);
return resolved;
}
Pavel
> }
>
> return real_realpath(path, resolved);
> --
> 2.26.3
>
Attachment:
signature.asc
Description: PGP signature
