[Mesa-dev] [PATCH] util/disk_cache: Fix disk_cache_get_function_timestamp with disabled cache.

[Eric Engestrom]

On Thursday, 2018-07-19 21:39:05 +0200, Bas Nieuwenhuizen wrote:
> On Thu, Jul 19, 2018 at 6:48 PM, Eric Engestrom
> <eric.engestrom at intel.com> wrote:
> > On Wednesday, 2018-07-18 14:01:49 +0200, Bas Nieuwenhuizen wrote:
> >> radv always needs it, so just check the header instead. Also
> >> do not declare the function if the variable is not set, so we
> >> get a nice compile error instead of failing to open a device
> >> at runtime.
> >
> > If that's the goal, why have any guards? Just #include the header,
> > that's your compilation error if it's missing :)
> 
> Well the goal is only to introduce a compile error if we do not have
> it, but any of the users of the function gets build. Notably this
> function only gets used in radv,radeonsi,r600g and nouveau, while the
> disk_cache.h gets included also from e.g. the glsl compiler which is
> an issue for e.g. windows.
> 
> radv works fine without the disk cache enabled, but still needs this
> function or it fails device creation (a non-disk cache is part of the
> vulkan API).

Oh right, you want to still be able to #include "disk_cache.h" in places
that don't need `disk_cache_get_function_timestamp()`.

Then this patch look correct to me; sorry for the confusion earlier.
Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>

> 
> >
> >>
> >> Fixes: b87ef9e606a "util: fix MSVC build issue in disk_cache.h"
> >> ---
> >>  configure.ac          | 1 +
> >>  meson.build           | 2 +-
> >>  src/util/disk_cache.h | 8 +++-----
> >>  3 files changed, 5 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/configure.ac b/configure.ac
> >> index c946454cfae..ffb8424a07b 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -872,6 +872,7 @@ AC_HEADER_MAJOR
> >>  AC_CHECK_HEADER([xlocale.h], [DEFINES="$DEFINES -DHAVE_XLOCALE_H"])
> >>  AC_CHECK_HEADER([sys/sysctl.h], [DEFINES="$DEFINES -DHAVE_SYS_SYSCTL_H"])
> >>  AC_CHECK_HEADERS([endian.h])
> >> +AC_CHECK_HEADER([dlfcn.h], [DEFINES="$DEFINES -DHAVE_DLFCN_H"])
> >>  AC_CHECK_FUNC([strtof], [DEFINES="$DEFINES -DHAVE_STRTOF"])
> >>  AC_CHECK_FUNC([mkostemp], [DEFINES="$DEFINES -DHAVE_MKOSTEMP"])
> >>  AC_CHECK_FUNC([timespec_get], [DEFINES="$DEFINES -DHAVE_TIMESPEC_GET"])
> >> diff --git a/meson.build b/meson.build
> >> index e05645cbf39..86a4a4ce6da 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -960,7 +960,7 @@ elif cc.has_header_symbol('sys/mkdev.h', 'major')
> >>    pre_args += '-DMAJOR_IN_MKDEV'
> >>  endif
> >>
> >> -foreach h : ['xlocale.h', 'sys/sysctl.h', 'linux/futex.h', 'endian.h']
> >> +foreach h : ['xlocale.h', 'sys/sysctl.h', 'linux/futex.h', 'endian.h', 'dlfcn.h']
> >>    if cc.compiles('#include <@0@>'.format(h), name : '@0@'.format(h))
> >>      pre_args += '-DHAVE_ at 0@'.format(h.to_upper().underscorify())
> >>    endif
> >> diff --git a/src/util/disk_cache.h b/src/util/disk_cache.h
> >> index f84840fb5ca..50bd9f41ac4 100644
> >> --- a/src/util/disk_cache.h
> >> +++ b/src/util/disk_cache.h
> >> @@ -24,7 +24,7 @@
> >>  #ifndef DISK_CACHE_H
> >>  #define DISK_CACHE_H
> >>
> >> -#ifdef ENABLE_SHADER_CACHE
> >> +#ifdef HAVE_DLFCN_H
> >>  #include <dlfcn.h>
> >>  #endif
> >>  #include <assert.h>
> >> @@ -88,10 +88,10 @@ disk_cache_format_hex_id(char *buf, const uint8_t *hex_id, unsigned size)
> >>     return buf;
> >>  }
> >>
> >> +#ifdef HAVE_DLFCN_H
> >>  static inline bool
> >>  disk_cache_get_function_timestamp(void *ptr, uint32_t* timestamp)
> >>  {
> >> -#ifdef ENABLE_SHADER_CACHE
> >>     Dl_info info;
> >>     struct stat st;
> >>     if (!dladdr(ptr, &info) || !info.dli_fname) {
> >> @@ -102,10 +102,8 @@ disk_cache_get_function_timestamp(void *ptr, uint32_t* timestamp)
> >>     }
> >>     *timestamp = st.st_mtime;
> >>     return true;
> >> -#else
> >> -   return false;
> >> -#endif
> >>  }
> >> +#endif
> >>
> >>  /* Provide inlined stub functions if the shader cache is disabled. */
> >>
> >> --
> >> 2.18.0
> >>
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev