Re: [libvirt PATCH 3/6] tests: virfilemock: realpath: Allow non-null second parameter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux