[Mesa-dev] [PATCH 1/4] util/disk_cache: rename mesa cache dir and introduce cache versioning

[Emil Velikov]

Hi Tim,

A couple of small suggestions which should be applicable, regardless
of the current discussion.

On 15 August 2017 at 00:26, Timothy Arceri <tarceri at itsqueeze.com> wrote:
> Steam is already analysing cache items, unfortunatly we did not
s/unfortunatly/unfortunately/


> -   check_directories_created(CACHE_TEST_TMP "/xdg-cache-home/mesa");
> +   check_directories_created(CACHE_TEST_TMP
> +                             "/xdg-cache-home/mesa_shader_cache");
>
Replace "mesa_shader_cache" instances with the define?

At some misc later stage:
 - introduce defines for xdg-cache-home and mesa-glsl-cache-dir


> +#define CACHE_DIR_NAME "mesa_shader_cache"
> +
> +/* The cache version should be bumped whenever a change is made to the
> + * structure of cache entries or the index. This will give any 3rd party
> + * applications reading the cache entries a chance to adjust to the changes.
> + */
I don't have strong opinion on each of the following, although I think
it's important to have them documented:
 - is the version checked in internally
 - is forward/backward compatibility expected

> +#define CACHE_VERSION 1
> +


> -   memcpy(cache->driver_keys_blob, timestamp, ts_size);
> -   memcpy(cache->driver_keys_blob + ts_size, gpu_name, gpu_name_size);
> -   memcpy(cache->driver_keys_blob + ts_size + gpu_name_size, &ptr_size,
> -          ptr_size_size);
> -   memcpy(cache->driver_keys_blob + ts_size + gpu_name_size + ptr_size_size,
> +   memcpy(cache->driver_keys_blob, &cache_version, cv_size);
> +   memcpy(cache->driver_keys_blob + cv_size, timestamp, ts_size);
> +   memcpy(cache->driver_keys_blob + cv_size + ts_size, gpu_name,
> +          gpu_name_size);
> +   memcpy(cache->driver_keys_blob + cv_size + ts_size + gpu_name_size,
> +          &ptr_size, ptr_size_size);
> +   memcpy(cache->driver_keys_blob + cv_size + ts_size +
> +          gpu_name_size + ptr_size_size,
>            &driver_flags, driver_flags_size);
>
In case you're not a huge fan of the memcpy(ptr + a + b + ... + z,
...) bits something like the following should help.

// One could also kill off _src_size
#define FOO_CPY(_dst, _src, _src_size) { \
   memcpy(_dst, _src, _src_size); \
   _dst += _src_size; \
} while (0)


-Emil