[Mesa-dev] [PATCH 03/65] glsl: Add initial functions to implement an on-disk cache

[Timothy Arceri]

On Sat, 2016-04-30 at 11:05 +0200, Erik Faye-Lund wrote:
> On Fri, Apr 29, 2016 at 3:33 PM, Timothy Arceri
> <timothy.arceri at collabora.com> wrote:
> > 
> > +/* Create a directory named 'path' if it does not already exist.
> > + *
> > + * Returns: 0 if path already exists as a directory or if created.
> > + *         -1 in all other cases.
> > + */
> > +static int
> > +mkdir_if_needed(char *path)
> > +{
> > +   struct stat sb;
> > +
> > +   /* If the path exists already, then our work is done if it's a
> > +    * directory, but it's an error if it is not.
> > +    */
> > +   if (stat(path, &sb) == 0) {
> > +      if (S_ISDIR(sb.st_mode)) {
> > +         return 0;
> > +      } else {
> > +         _mesa_warning(NULL,
> > +                       "Cannot use %s for shader cache (not a
> > directory)"
> > +                       "---disabling.\n", path);
> > +         return -1;
> > +      }
> > +   }
> > +
> > +   if (mkdir(path, 0755) == 0)
> > +      return 0;
> > +
> > +   _mesa_warning(NULL,
> > +                 "Failed to create %s for shader cache (%s)---
> > disabling.\n",
> > +                 path, strerror(errno));
> > +
> > +   return -1;
> > +}
> > +
> <snip>
> > 
> > +   /* Determine path for cache based on the first defined name as
> > follows:
> > +    *
> > +    *   $MESA_GLSL_CACHE_DIR
> > +    *   $XDG_CACHE_HOME/mesa
> > +    *   <pwd.pw_dir>/.cache/mesa
> > +    */
> > +   path = getenv("MESA_GLSL_CACHE_DIR");
> > +   if (path && mkdir_if_needed(path) == -1) {
> > +      goto fail;
> > +   }
> > +
> Hmm, doesn't this only create the last path component (if
> nonexistent)? Shouldn't we create *all* leading directories in this
> case?

Hi,

Sorry went on holiday and have only just got back to this.

You are correct this will not create leading directories. Besides
rebasing and some clean ups I didn't write anything in this patch so
I'm not sure what the thinking was but there is a test that
specifically checks this doesn't create leading directories. 

For now I think I'll leave it as is and try to get this stuff committed
before working on making it nicer.

Thanks for taking a look,
Tim

> 
> I would have expected to see something along the lines of this:
> 
> static int create_leading_dirs(const char *base)
> {
>    char *pos, buf[FILENAME_MAX];
> 
>    strncpy(buf, base, sizeof(buf));
>    buf[sizeof(buf) - 1] = '\0';
>    pos = buf + (*buf == '/');
> 
>    while (1) {
>       struct stat st;
> 
>       pos = strchr(pos, '/');
>       if (!pos)
>          break;
>       *pos = '\0';
> 
>       if (!stat(buf, &st)) {
>          if (!S_ISDIR(st.st_mode))
>             return -1;
>       } else {
>          if (mkdir(buf, 0777))
>             return -1;
>       }
> 
>       *pos++ = '/';
>    }
> 
>    return 0;
> }