[Mesa-dev] [PATCH 6/7] glsl/cache.c: Dynamically allocate filenames internal to cache

[Carl Worth]

The user can put the cache directory anywhere, so it's not safe to use
fixed-size arrays to store filenames. Instead, allocate the cache
pointer itself as a ralloc context and use that to dynamically
allocate all filenames.

While making this change, simplify the error handling in cache_get
with a new "goto FAIL" block so the cleanup code exists in a single
place, rather than being spread throughout the function over and over.
---
 src/glsl/cache.c | 87 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 54 insertions(+), 33 deletions(-)

diff --git a/src/glsl/cache.c b/src/glsl/cache.c
index 79e8afb..0bbf659 100644
--- a/src/glsl/cache.c
+++ b/src/glsl/cache.c
@@ -197,14 +197,14 @@ cache_create(void)
       fsync(fd);
    }
 
-   cache = (struct program_cache *) malloc(sizeof *cache);
+   cache = ralloc(NULL, struct program_cache);
    if (cache == NULL) {
       goto done;
    }
 
    cache->path = strdup(path);
    if (cache->path == NULL) {
-      free (cache);
+      ralloc_free (cache);
       cache = NULL;
       goto done;
    }
@@ -214,7 +214,7 @@ cache_create(void)
    cache->index = (unsigned char *)
       mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
    if (cache->index == MAP_FAILED) {
-      free(cache);
+      ralloc_free(cache);
       cache = NULL;
       goto done;
    }
@@ -227,16 +227,18 @@ cache_create(void)
    return cache;
 }
 
+/* Return a filename within the cache's directory corresponding to 'key'. The
+ * returned filename is ralloced with 'cache' as the parent context.
+ *
+ * Returns NULL if out of memory.
+ */
 static char *
-get_cache_file(struct program_cache *cache,
-               char *buffer, size_t size, cache_key key)
+get_cache_file(struct program_cache *cache, cache_key key)
 {
    char buf[41];
 
-   snprintf(buffer, size, "%s/%s",
-            cache->path, _mesa_sha1_format(buf, key));
-
-   return buffer;
+   return ralloc_asprintf(cache, "%s/%s", cache->path,
+                          _mesa_sha1_format(buf, key));
 }
 
 int
@@ -261,59 +263,69 @@ cache_has(struct program_cache *cache, cache_key key)
 uint8_t *
 cache_get(struct program_cache *cache, cache_key key, size_t *size)
 {
-   int fd, ret, len;
+   int fd = -1, ret, len;
    struct stat sb;
-   char filename[256], *data;
+   char *filename = NULL;
+   uint8_t *data = NULL;
 
    if (size)
       *size = 0;
 
    if (!cache_has(cache, key))
-      return NULL;
+      goto fail;
 
-   get_cache_file(cache, filename, sizeof filename, key);
+   filename = get_cache_file(cache, key);
+   if (filename == NULL)
+      goto fail;
 
    fd = open(filename, O_RDONLY | O_CLOEXEC);
    if (fd == -1)
-      return NULL;
+      goto fail;
 
-   if (fstat(fd, &sb) == -1) {
-      close(fd);
-      return NULL;
-   }
+   if (fstat(fd, &sb) == -1)
+      goto fail;
 
-   data = (char *) malloc(sb.st_size);
-   if (data == NULL) {
-      close(fd);
-      return NULL;
-   }
+   data = malloc(sb.st_size);
+   if (data == NULL)
+      goto fail;
 
    for (len = 0; len < sb.st_size; len += ret) {
       ret = read(fd, data + len, sb.st_size - len);
-      if (ret == -1) {
-         free(data);
-         close(fd);
-         return NULL;
-      }
+      if (ret == -1)
+         goto fail;
    }
 
+   ralloc_free(filename);
    close(fd);
 
    if (size)
       *size = sb.st_size;
 
-   return (void *) data;
+   return data;
+
+ fail:
+   if (data)
+      free(data);
+   if (filename)
+      ralloc_free(filename);
+   if (fd != -1)
+      close(fd);
+
+   return NULL;
 }
 
 void
-cache_put(struct program_cache *cache, cache_key key, const void *data, size_t size)
+cache_put(struct program_cache *cache,
+          cache_key key,
+          const void *data,
+          size_t size)
 {
    uint32_t *s = (uint32_t *) key;
    int i = *s & (INDEX_SIZE - 1);
    unsigned char *entry;
    int fd, ret;
    size_t len;
-   char filename[256];
+   char *filename;
    const char *p = (const char *) data;
 
    /* FIXME: We'll need an fsync here and think about races... maybe even need
@@ -322,8 +334,13 @@ cache_put(struct program_cache *cache, cache_key key, const void *data, size_t s
     * it. */
 
    entry = &cache->index[i * CACHE_KEY_SIZE];
-   get_cache_file(cache, filename, sizeof filename, entry);
+   filename = get_cache_file(cache, entry);
+   if (filename == NULL)
+      return;
+
    unlink(filename);
+   ralloc_free(filename);
+
    memcpy(entry, key, CACHE_KEY_SIZE);
 
    if (data == NULL)
@@ -335,8 +352,12 @@ cache_put(struct program_cache *cache, cache_key key, const void *data, size_t s
     * same file is ok, since they'll both write the same contents, and whoever
     * finishes first will move the complete file in place. */
 
-   get_cache_file(cache, filename, sizeof filename, key);
+   filename = get_cache_file(cache, key);
+   if (filename == NULL)
+      return;
+
    fd = open(filename, O_WRONLY | O_CLOEXEC | O_CREAT, 0644);
+   ralloc_free(filename);
    if (fd == -1)
       return;
 
-- 
2.1.4