use struct strbuf instead of static buffers

Use "struct strbuf" from Git to remove the limit on file path length.

Notes on scan-tree:
This is slightly involved since I decided to pass the strbuf into
add_repo() and modify if whenever a new file name is required, which
should avoid any extra allocations within that function.  The pattern
there is to append the filename, use it and then reset the buffer to its
original length (retaining a trailing '/').

Notes on ui-snapshot:
Since write_archive modifies the argv array passed to it we
copy the argv_array values into a new array of char* and then free the
original argv_array structure and the new array without worrying about
what the values now look like.

Signed-off-by: John Keeping <john@keeping.me.uk>

12 files changed, 305 insertions, 243 deletions

diff --git a/cache.c b/cache.c
index 3127fc2..c1d777b 100644
--- a/cache.c
+++ b/cache.c
@@ -312,9 +312,9 @@ int cache_process(int size, const char *path, const char *key, int ttl,
                   cache_fill_fn fn, void *cbdata)
 {
         unsigned long hash;
-        int len, i;
-        char filename[1024];
-        char lockname[1024 + 5];  /* 5 = ".lock" */
+        int i;
+        struct strbuf filename = STRBUF_INIT;
+        struct strbuf lockname = STRBUF_INIT;
         struct cache_slot slot;
 
         /* If the cache is disabled, just generate the content */
@@ -329,32 +329,22 @@ int cache_process(int size, const char *path, const char *key, int ttl,
                 fn(cbdata);
                 return 0;
         }
-        len = strlen(path);
-        if (len > sizeof(filename) - 10) { /* 10 = "/01234567\0" */
-                cache_log("[cgit] Cache path too long, caching is disabled: %s\n",
-                          path);
-                fn(cbdata);
-                return 0;
-        }
         if (!key)
                 key = "";
         hash = hash_str(key) % size;
-        strcpy(filename, path);
-        if (filename[len - 1] != '/')
-                filename[len++] = '/';
+        strbuf_addstr(&filename, path);
+        strbuf_ensure_end(&filename, '/');
         for (i = 0; i < 8; i++) {
-                sprintf(filename + len++, "%x",
-                        (unsigned char)(hash & 0xf));
+                strbuf_addf(&filename, "%x", (unsigned char)(hash & 0xf));
                 hash >>= 4;
         }
-        filename[len] = '\0';
-        strcpy(lockname, filename);
-        strcpy(lockname + len, ".lock");
+        strbuf_addbuf(&lockname, &filename);
+        strbuf_addstr(&lockname, ".lock");
         slot.fn = fn;
         slot.cbdata = cbdata;
         slot.ttl = ttl;
-        slot.cache_name = filename;
-        slot.lock_name = lockname;
+        slot.cache_name = strbuf_detach(&filename, NULL);
+        slot.lock_name = strbuf_detach(&lockname, NULL);
         slot.key = key;
         slot.keylen = strlen(key);
         return process_slot(&slot);
@@ -381,18 +371,13 @@ int cache_ls(const char *path)
         struct dirent *ent;
         int err = 0;
         struct cache_slot slot;
-        char fullname[1024];
-        char *name;
+        struct strbuf fullname = STRBUF_INIT;
+        size_t prefixlen;
 
         if (!path) {
                 cache_log("[cgit] cache path not specified\n");
                 return -1;
         }
-        if (strlen(path) > 1024 - 10) {
-                cache_log("[cgit] cache path too long: %s\n",
-                          path);
-                return -1;
-        }
         dir = opendir(path);
         if (!dir) {
                 err = errno;
@@ -400,30 +385,28 @@ int cache_ls(const char *path)
                           path, strerror(err), err);
                 return err;
         }
-        strcpy(fullname, path);
-        name = fullname + strlen(path);
-        if (*(name - 1) != '/') {
-                *name++ = '/';
-                *name = '\0';
-        }
-        slot.cache_name = fullname;
+        strbuf_addstr(&fullname, path);
+        strbuf_ensure_end(&fullname, '/');
+        prefixlen = fullname.len;
         while ((ent = readdir(dir)) != NULL) {
                 if (strlen(ent->d_name) != 8)
                         continue;
-                strcpy(name, ent->d_name);
+                strbuf_setlen(&fullname, prefixlen);
+                strbuf_addstr(&fullname, ent->d_name);
                 if ((err = open_slot(&slot)) != 0) {
                         cache_log("[cgit] unable to open path %s: %s (%d)\n",
-                                  fullname, strerror(err), err);
+                                  fullname.buf, strerror(err), err);
                         continue;
                 }
                 printf("%s %s %10"PRIuMAX" %s\n",
-                       name,
+                       fullname.buf,
                        sprintftime("%Y-%m-%d %H:%M:%S",
                                    slot.cache_st.st_mtime),
                        (uintmax_t)slot.cache_st.st_size,
                        slot.buf);
                 close_slot(&slot);
         }
+        slot.cache_name = strbuf_detach(&fullname, NULL);
         closedir(dir);
         return 0;
 }
diff --git a/cgit.c b/cgit.c
index 4e51283..f73c7b0 100644
--- a/cgit.c
+++ b/cgit.c
@@ -468,8 +468,8 @@ static int prepare_repo_cmd(struct cgit_context *ctx)
         if (nongit) {
                 const char *name = ctx->repo->name;
                 rc = errno;
-                ctx->page.title = fmt("%s - %s", ctx->cfg.root_title,
-                                      "config error");
+                ctx->page.title = fmtalloc("%s - %s", ctx->cfg.root_title,
+                                                "config error");
                 ctx->repo = NULL;
                 cgit_print_http_headers(ctx);
                 cgit_print_docstart(ctx);
@@ -479,7 +479,7 @@ static int prepare_repo_cmd(struct cgit_context *ctx)
                 cgit_print_docend();
                 return 1;
         }
-        ctx->page.title = fmt("%s - %s", ctx->repo->name, ctx->repo->desc);
+        ctx->page.title = fmtalloc("%s - %s", ctx->repo->name, ctx->repo->desc);
 
         if (!ctx->repo->defbranch)
                 ctx->repo->defbranch = guess_defbranch();
@@ -577,21 +577,16 @@ static int cmp_repos(const void *a, const void *b)
 static char *build_snapshot_setting(int bitmap)
 {
         const struct cgit_snapshot_format *f;
-        char *result = xstrdup("");
-        char *tmp;
-        int len;
+        struct strbuf result = STRBUF_INIT;
 
         for (f = cgit_snapshot_formats; f->suffix; f++) {
                 if (f->bit & bitmap) {
-                        tmp = result;
-                        result = xstrdup(fmt("%s%s ", tmp, f->suffix));
-                        free(tmp);
+                        if (result.len)
+                                strbuf_addch(&result, ' ');
+                        strbuf_addstr(&result, f->suffix);
                 }
         }
-        len = strlen(result);
-        if (len)
-                result[len - 1] = '\0';
-        return result;
+        return strbuf_detach(&result, NULL);
 }
 
 static char *get_first_line(char *txt)
@@ -639,7 +634,7 @@ static void print_repo(FILE *f, struct cgit_repo *repo)
                 fprintf(f, "repo.source-filter=%s\n", repo->source_filter->cmd);
         if (repo->snapshots != ctx.cfg.snapshots) {
                 char *tmp = build_snapshot_setting(repo->snapshots);
-                fprintf(f, "repo.snapshots=%s\n", tmp);
+                fprintf(f, "repo.snapshots=%s\n", tmp ? tmp : "");
                 free(tmp);
         }
         if (repo->max_stats != ctx.cfg.max_stats)
@@ -661,20 +656,22 @@ static void print_repolist(FILE *f, struct cgit_repolist *list, int start)
  */
 static int generate_cached_repolist(const char *path, const char *cached_rc)
 {
-        char *locked_rc;
+        struct strbuf locked_rc = STRBUF_INIT;
+        int result = 0;
         int idx;
         FILE *f;
 
-        locked_rc = xstrdup(fmt("%s.lock", cached_rc));
-        f = fopen(locked_rc, "wx");
+        strbuf_addf(&locked_rc, "%s.lock", cached_rc);
+        f = fopen(locked_rc.buf, "wx");
         if (!f) {
                 /* Inform about the error unless the lockfile already existed,
                  * since that only means we've got concurrent requests.
                  */
-                if (errno != EEXIST)
+                result = errno;
+                if (result != EEXIST)
                         fprintf(stderr, "[cgit] Error opening %s: %s (%d)\n",
-                                locked_rc, strerror(errno), errno);
-                return errno;
+                                locked_rc.buf, strerror(result), result);
+                goto out;
         }
         idx = cgit_repolist.count;
         if (ctx.cfg.project_list)
@@ -682,55 +679,59 @@ static int generate_cached_repolist(const char *path, const char *cached_rc)
         else
                 scan_tree(path, repo_config);
         print_repolist(f, &cgit_repolist, idx);
-        if (rename(locked_rc, cached_rc))
+        if (rename(locked_rc.buf, cached_rc))
                 fprintf(stderr, "[cgit] Error renaming %s to %s: %s (%d)\n",
-                        locked_rc, cached_rc, strerror(errno), errno);
+                        locked_rc.buf, cached_rc, strerror(errno), errno);
         fclose(f);
-        return 0;
+out:
+        strbuf_release(&locked_rc);
+        return result;
 }
 
 static void process_cached_repolist(const char *path)
 {
         struct stat st;
-        char *cached_rc;
+        struct strbuf cached_rc = STRBUF_INIT;
         time_t age;
         unsigned long hash;
 
         hash = hash_str(path);
         if (ctx.cfg.project_list)
                 hash += hash_str(ctx.cfg.project_list);
-        cached_rc = xstrdup(fmt("%s/rc-%8lx", ctx.cfg.cache_root, hash));
+        strbuf_addf(&cached_rc, "%s/rc-%8lx", ctx.cfg.cache_root, hash);
 
-        if (stat(cached_rc, &st)) {
+        if (stat(cached_rc.buf, &st)) {
                 /* Nothing is cached, we need to scan without forking. And
                  * if we fail to generate a cached repolist, we need to
                  * invoke scan_tree manually.
                  */
-                if (generate_cached_repolist(path, cached_rc)) {
+                if (generate_cached_repolist(path, cached_rc.buf)) {
                         if (ctx.cfg.project_list)
                                 scan_projects(path, ctx.cfg.project_list,
                                               repo_config);
                         else
                                 scan_tree(path, repo_config);
                 }
-                return;
+                goto out;
         }
 
-        parse_configfile(cached_rc, config_cb);
+        parse_configfile(cached_rc.buf, config_cb);
 
         /* If the cached configfile hasn't expired, lets exit now */
         age = time(NULL) - st.st_mtime;
         if (age <= (ctx.cfg.cache_scanrc_ttl * 60))
-                return;
+                goto out;
 
         /* The cached repolist has been parsed, but it was old. So lets
          * rescan the specified path and generate a new cached repolist
          * in a child-process to avoid latency for the current request.
          */
         if (fork())
-                return;
+                goto out;
 
-        exit(generate_cached_repolist(path, cached_rc));
+        exit(generate_cached_repolist(path, cached_rc.buf));
+out:
+        strbuf_release(&cached_rc);
 }
 
 static void cgit_parse_args(int argc, const char **argv)
@@ -812,7 +813,6 @@ static int calc_ttl()
 int main(int argc, const char **argv)
 {
         const char *path;
-        char *qry;
         int err, ttl;
 
         prepare_context(&ctx);
@@ -843,9 +843,9 @@ int main(int argc, const char **argv)
                         path++;
                 ctx.qry.url = xstrdup(path);
                 if (ctx.qry.raw) {
-                        qry = ctx.qry.raw;
-                        ctx.qry.raw = xstrdup(fmt("%s?%s", path, qry));
-                        free(qry);
+                        char *newqry = fmtalloc("%s?%s", path, ctx.qry.raw);
+                        free(ctx.qry.raw);
+                        ctx.qry.raw = newqry;
                 } else
                         ctx.qry.raw = xstrdup(ctx.qry.url);
                 cgit_parse_url(ctx.qry.url);
diff --git a/scan-tree.c b/scan-tree.c
index 05caba5..beb584b 100644
--- a/scan-tree.c
+++ b/scan-tree.c
@@ -12,38 +12,38 @@
 #include "configfile.h"
 #include "html.h"
 
-#define MAX_PATH 4096
-
 /* return 1 if path contains a objects/ directory and a HEAD file */
 static int is_git_dir(const char *path)
 {
         struct stat st;
-        static char buf[MAX_PATH];
+        struct strbuf pathbuf = STRBUF_INIT;
+        int result = 0;
 
-        if (snprintf(buf, MAX_PATH, "%s/objects", path) >= MAX_PATH) {
-                fprintf(stderr, "Insanely long path: %s\n", path);
-                return 0;
-        }
-        if (stat(buf, &st)) {
+        strbuf_addf(&pathbuf, "%s/objects", path);
+        if (stat(pathbuf.buf, &st)) {
                 if (errno != ENOENT)
                         fprintf(stderr, "Error checking path %s: %s (%d)\n",
                                 path, strerror(errno), errno);
-                return 0;
+                goto out;
         }
         if (!S_ISDIR(st.st_mode))
-                return 0;
+                goto out;
 
-        sprintf(buf, "%s/HEAD", path);
-        if (stat(buf, &st)) {
+        strbuf_reset(&pathbuf);
+        strbuf_addf(&pathbuf, "%s/HEAD", path);
+        if (stat(pathbuf.buf, &st)) {
                 if (errno != ENOENT)
                         fprintf(stderr, "Error checking path %s: %s (%d)\n",
                                 path, strerror(errno), errno);
-                return 0;
+                goto out;
         }
         if (!S_ISREG(st.st_mode))
-                return 0;
+                goto out;
 
-        return 1;
+        result = 1;
+out:
+        strbuf_release(&pathbuf);
+        return result;
 }
 
 struct cgit_repo *repo;
@@ -75,47 +75,61 @@ static char *xstrrchr(char *s, char *from, int c)
         return from < s ? NULL : from;
 }
 
-static void add_repo(const char *base, const char *path, repo_config_fn fn)
+static void add_repo(const char *base, struct strbuf *path, repo_config_fn fn)
 {
         struct stat st;
         struct passwd *pwd;
-        char *rel, *p, *slash;
+        size_t pathlen;
+        struct strbuf rel = STRBUF_INIT;
+        char *p, *slash;
         int n;
         size_t size;
 
-        if (stat(path, &st)) {
+        if (stat(path->buf, &st)) {
                 fprintf(stderr, "Error accessing %s: %s (%d)\n",
-                        path, strerror(errno), errno);
+                        path->buf, strerror(errno), errno);
                 return;
         }
 
-        if (ctx.cfg.strict_export && stat(fmt("%s/%s", path, ctx.cfg.strict_export), &st))
-                return;
+        strbuf_addch(path, '/');
+        pathlen = path->len;
 
-        if (!stat(fmt("%s/noweb", path), &st))
+        if (ctx.cfg.strict_export) {
+                strbuf_addstr(path, ctx.cfg.strict_export);
+                if(stat(path->buf, &st))
+                        return;
+                strbuf_setlen(path, pathlen);
+        }
+
+        strbuf_addstr(path, "noweb");
+        if (!stat(path->buf, &st))
                 return;
+        strbuf_setlen(path, pathlen);
 
-        if (base == path)
-                rel = xstrdup(path);
+        if (strncmp(base, path->buf, strlen(base)))
+                strbuf_addbuf(&rel, path);
         else
-                rel = xstrdup(path + strlen(base) + 1);
+                strbuf_addstr(&rel, path->buf + strlen(base) + 1);
 
-        if (!strcmp(rel + strlen(rel) - 5, "/.git"))
-                rel[strlen(rel) - 5] = '\0';
+        if (!strcmp(rel.buf + rel.len - 5, "/.git"))
+                strbuf_setlen(&rel, rel.len - 5);
 
-        repo = cgit_add_repo(rel);
+        repo = cgit_add_repo(rel.buf);
         config_fn = fn;
-        if (ctx.cfg.enable_git_config)
-                git_config_from_file(gitconfig_config, fmt("%s/config", path), NULL);
+        if (ctx.cfg.enable_git_config) {
+                strbuf_addstr(path, "config");
+                git_config_from_file(gitconfig_config, path->buf, NULL);
+                strbuf_setlen(path, pathlen);
+        }
 
         if (ctx.cfg.remove_suffix)
                 if ((p = strrchr(repo->url, '.')) && !strcmp(p, ".git"))
                         *p = '\0';
-        repo->path = xstrdup(path);
+        repo->path = xstrdup(path->buf);
         while (!repo->owner) {
                 if ((pwd = getpwuid(st.st_uid)) == NULL) {
                         fprintf(stderr, "Error reading owner-info for %s: %s (%d)\n",
-                                path, strerror(errno), errno);
+                                path->buf, strerror(errno), errno);
                         break;
                 }
                 if (pwd->pw_gecos)
@@ -125,30 +139,32 @@ static void add_repo(const char *base, const char *path, repo_config_fn fn)
         }
 
         if (repo->desc == cgit_default_repo_desc || !repo->desc) {
-                p = fmt("%s/description", path);
-                if (!stat(p, &st))
-                        readfile(p, &repo->desc, &size);
+                strbuf_addstr(path, "description");
+                if (!stat(path->buf, &st))
+                        readfile(path->buf, &repo->desc, &size);
+                strbuf_setlen(path, pathlen);
         }
 
         if (!repo->readme) {
-                p = fmt("%s/README.html", path);
-                if (!stat(p, &st))
+                strbuf_addstr(path, "README.html");
+                if (!stat(path->buf, &st))
                         repo->readme = "README.html";
+                strbuf_setlen(path, pathlen);
         }
         if (ctx.cfg.section_from_path) {
                 n  = ctx.cfg.section_from_path;
                 if (n > 0) {
-                        slash = rel;
+                        slash = rel.buf;
                         while (slash && n && (slash = strchr(slash, '/')))
                                 n--;
                 } else {
-                        slash = rel + strlen(rel);
-                        while (slash && n && (slash = xstrrchr(rel, slash, '/')))
+                        slash = rel.buf + rel.len;
+                        while (slash && n && (slash = xstrrchr(rel.buf, slash, '/')))
                                 n++;
                 }
                 if (slash && !n) {
                         *slash = '\0';
-                        repo->section = xstrdup(rel);
+                        repo->section = xstrdup(rel.buf);
                         *slash = '/';
                         if (!prefixcmp(repo->name, repo->section)) {
                                 repo->name += strlen(repo->section);
@@ -158,19 +174,19 @@ static void add_repo(const char *base, const char *path, repo_config_fn fn)
                 }
         }
 
-        p = fmt("%s/cgitrc", path);
-        if (!stat(p, &st))
-                parse_configfile(xstrdup(p), &repo_config);
-
+        strbuf_addstr(path, "cgitrc");
+        if (!stat(path->buf, &st))
+                parse_configfile(xstrdup(path->buf), &repo_config);
 
-        free(rel);
+        strbuf_release(&rel);
 }
 
 static void scan_path(const char *base, const char *path, repo_config_fn fn)
 {
         DIR *dir = opendir(path);
         struct dirent *ent;
-        char *buf;
+        struct strbuf pathbuf = STRBUF_INIT;
+        size_t pathlen = strlen(path);
         struct stat st;
 
         if (!dir) {
@@ -178,14 +194,22 @@ static void scan_path(const char *base, const char *path, repo_config_fn fn)
                         path, strerror(errno), errno);
                 return;
         }
-        if (is_git_dir(path)) {
-                add_repo(base, path, fn);
+
+        strbuf_add(&pathbuf, path, strlen(path));
+        if (is_git_dir(pathbuf.buf)) {
+                add_repo(base, &pathbuf, fn);
                 goto end;
         }
-        if (is_git_dir(fmt("%s/.git", path))) {
-                add_repo(base, fmt("%s/.git", path), fn);
+        strbuf_addstr(&pathbuf, "/.git");
+        if (is_git_dir(pathbuf.buf)) {
+                add_repo(base, &pathbuf, fn);
                 goto end;
         }
+        /*
+         * Add one because we don't want to lose the trailing '/' when we
+         * reset the length of pathbuf in the loop below.
+         */
+        pathlen++;
         while ((ent = readdir(dir)) != NULL) {
                 if (ent->d_name[0] == '.') {
                         if (ent->d_name[1] == '\0')
@@ -195,24 +219,18 @@ static void scan_path(const char *base, const char *path, repo_config_fn fn)
                         if (!ctx.cfg.scan_hidden_path)
                                 continue;
                 }
-                buf = malloc(strlen(path) + strlen(ent->d_name) + 2);
-                if (!buf) {
-                        fprintf(stderr, "Alloc error on %s: %s (%d)\n",
-                                path, strerror(errno), errno);
-                        exit(1);
-                }
-                sprintf(buf, "%s/%s", path, ent->d_name);
-                if (stat(buf, &st)) {
+                strbuf_setlen(&pathbuf, pathlen);
+                strbuf_addstr(&pathbuf, ent->d_name);
+                if (stat(pathbuf.buf, &st)) {
                         fprintf(stderr, "Error checking path %s: %s (%d)\n",
-                                buf, strerror(errno), errno);
-                        free(buf);
+                                pathbuf.buf, strerror(errno), errno);
                         continue;