Author: John Keeping <john@keeping.me.uk>
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>
cache.c | 57 ++++++------------ cgit.c | 72 +++++++++++----------- scan-tree.c | 160 +++++++++++++++++++++++++++++----------------------- ui-log.c | 33 +++++++--- ui-plain.c | 6 + ui-refs.c | 10 +- ui-repolist.c | 28 +++++--- ui-shared.c | 63 +++++++++++--------- ui-snapshot.c | 60 ++++++++++++++----- ui-summary.c | 12 ++- ui-tag.c | 14 ++- ui-tree.c | 33 +++++-----
diff --git a/cache.c b/cache.c index 3127fc26366ba35363f81abc35cf579f07e7d612..c1d777bc0793c081cec662cc26fa36ea59e563bf 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 @@ cache_log("[cgit] Cache path not specified, caching is disabled\n"); 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 @@ DIR *dir; 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 @@ cache_log("[cgit] unable to open path %s: %s (%d)\n", 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 4e512838d480e5640e3a9758bf6a764471c46349..f73c7b0b6c7b76d3aa231fe75edfc2ac6acd4745 100644 --- a/cgit.c +++ b/cgit.c @@ -468,8 +468,8 @@ setup_git_directory_gently(&nongit); 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 @@ rc ? strerror(rc) : "Not a valid git repository"); 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 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 @@ if (repo->source_filter && repo->source_filter != ctx.cfg.source_filter) 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 @@ * and return 0 on success. */ 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 @@ scan_projects(path, ctx.cfg.project_list, repo_config); 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 @@ int main(int argc, const char **argv) { const char *path; - char *qry; int err, ttl; prepare_context(&ctx); @@ -843,9 +843,9 @@ if (path[0] == '/') 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 05caba53af0a88e06e1bbc73c1c34294f8a447db..beb584b5952eb9407e90fc8c580ffffdb5dc5676 100644 --- a/scan-tree.c +++ b/scan-tree.c @@ -12,38 +12,38 @@ #include "scan-tree.h" #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 @@ from--; 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 @@ repo->owner = xstrdup(pwd->pw_gecos ? pwd->pw_gecos : pwd->pw_name); } 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 @@ } } } - 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 @@ fprintf(stderr, "Error opening directory %s: %s (%d)\n", 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 @@ continue; 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; } if (S_ISDIR(st.st_mode)) - scan_path(base, buf, fn); - free(buf); + scan_path(base, pathbuf.buf, fn); } end: + strbuf_release(&pathbuf); closedir(dir); } @@ -220,7 +238,7 @@ #define lastc(s) s[strlen(s) - 1] void scan_projects(const char *path, const char *projectsfile, repo_config_fn fn) { - char line[MAX_PATH * 2], *z; + struct strbuf line = STRBUF_INIT; FILE *projects; int err; @@ -230,19 +248,19 @@ fprintf(stderr, "Error opening projectsfile %s: %s (%d)\n", projectsfile, strerror(errno), errno); return; } - while (fgets(line, sizeof(line), projects) != NULL) { - for (z = &lastc(line); - strlen(line) && strchr("\n\r", *z); - z = &lastc(line)) - *z = '\0'; - if (strlen(line)) - scan_path(path, fmt("%s/%s", path, line), fn); + while (strbuf_getline(&line, projects, '\n') != EOF) { + if (!line.len) + continue; + strbuf_insert(&line, 0, "/", 1); + strbuf_insert(&line, 0, path, strlen(path)); + scan_path(path, line.buf, fn); } if ((err = ferror(projects))) { fprintf(stderr, "Error reading from projectsfile %s: %s (%d)\n", projectsfile, strerror(err), err); } fclose(projects); + strbuf_release(&line); } void scan_tree(const char *path, repo_config_fn fn) diff --git a/ui-log.c b/ui-log.c index 8592843cea8f943552650145deb76450988343dc..93af0cee4d5f39c18e3e4727b8662b9f197f8391 100644 --- a/ui-log.c +++ b/ui-log.c @@ -243,15 +243,19 @@ strbuf_release(&graphbuf); cgit_free_commitinfo(info); } -static const char *disambiguate_ref(const char *ref) +static const char *disambiguate_ref(const char *ref, int *must_free_result) { unsigned char sha1[20]; - const char *longref; + struct strbuf longref = STRBUF_INIT; - longref = fmt("refs/heads/%s", ref); - if (get_sha1(longref, sha1) == 0) - return longref; + strbuf_addf(&longref, "refs/heads/%s", ref); + if (get_sha1(longref.buf, sha1) == 0) { + *must_free_result = 1; + return strbuf_detach(&longref, NULL); + } + *must_free_result = 0; + strbuf_release(&longref); return ref; } @@ -284,24 +288,26 @@ struct rev_info rev; struct commit *commit; struct vector vec = VECTOR_INIT(char *); int i, columns = commit_graph ? 4 : 3; - char *arg; + int must_free_tip = 0; + struct strbuf argbuf = STRBUF_INIT; /* First argv is NULL */ vector_push(&vec, NULL, 0); if (!tip) tip = ctx.qry.head; - tip = disambiguate_ref(tip); + tip = disambiguate_ref(tip, &must_free_tip); vector_push(&vec, &tip, 0); if (grep && pattern && *pattern) { pattern = xstrdup(pattern); if (!strcmp(grep, "grep") || !strcmp(grep, "author") || !strcmp(grep, "committer")) { - arg = fmt("--%s=%s", grep, pattern); - vector_push(&vec, &arg, 0); + strbuf_addf(&argbuf, "--%s=%s", grep, pattern); + vector_push(&vec, &argbuf.buf, 0); } if (!strcmp(grep, "range")) { + char *arg; /* Split the pattern at whitespace and add each token * as a revision expression. Do not accept other * rev-list options. Also, replace the previously @@ -336,8 +342,8 @@ vector_push(&vec, &topo_order_arg, 0); } if (path) { - arg = "--"; - vector_push(&vec, &arg, 0); + static const char *double_dash_arg = "--"; + vector_push(&vec, &double_dash_arg, 0); vector_push(&vec, &path, 0); } @@ -430,4 +436,9 @@ cgit_log_link("[...]", NULL, NULL, ctx.qry.head, NULL, ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg); html("</td></tr>\n"); } + + /* If we allocated tip then it is safe to cast away const. */ + if (must_free_tip) + free((char*) tip); + strbuf_release(&argbuf); } diff --git a/ui-plain.c b/ui-plain.c index 6b0d84b7a0aec7c8762227edf4c35899d2bd2cce..9c865423fdddd58a0777c50f05c559ea8a277dce 100644 --- a/ui-plain.c +++ b/ui-plain.c @@ -109,9 +109,9 @@ static char *buildpath(const char *base, int baselen, const char *path) { if (path[0]) - return fmt("%.*s%s/", baselen, base, path); + return fmtalloc("%.*s%s/", baselen, base, path); else - return fmt("%.*s/", baselen, base); + return fmtalloc("%.*s/", baselen, base); } static void print_dir(const unsigned char *sha1, const char *base, @@ -142,6 +142,7 @@ cgit_plain_link("../", NULL, NULL, ctx.qry.head, ctx.qry.sha1, fullpath); html("</li>\n"); } + free(fullpath); } static void print_dir_entry(const unsigned char *sha1, const char *base, @@ -159,6 +160,7 @@ } else cgit_plain_link(path, NULL, NULL, ctx.qry.head, ctx.qry.sha1, fullpath); html("</li>\n"); + free(fullpath); } static void print_dir_tail(void) diff --git a/ui-refs.c b/ui-refs.c index 74064789ecdd281bd411472b20730501d1c52182..3fbaad07302d90e1cb127666c51deceb67f5c02d 100644 --- a/ui-refs.c +++ b/ui-refs.c @@ -99,7 +99,7 @@ static void print_tag_downloads(const struct cgit_repo *repo, const char *ref) { const struct cgit_snapshot_format* f; - char *filename; + struct strbuf filename = STRBUF_INIT; const char *basename; int free_ref = 0; @@ -111,7 +111,7 @@ if (prefixcmp(ref, basename) != 0) { if ((ref[0] == 'v' || ref[0] == 'V') && isdigit(ref[1])) ref++; if (isdigit(ref[0])) { - ref = xstrdup(fmt("%s-%s", basename, ref)); + ref = fmtalloc("%s-%s", basename, ref); free_ref = 1; } } @@ -119,13 +119,15 @@ for (f = cgit_snapshot_formats; f->suffix; f++) { if (!(repo->snapshots & f->bit)) continue; - filename = fmt("%s%s", ref, f->suffix); - cgit_snapshot_link(filename, NULL, NULL, NULL, NULL, filename); + strbuf_reset(&filename); + strbuf_addf(&filename, "%s%s", ref, f->suffix); + cgit_snapshot_link(filename.buf, NULL, NULL, NULL, NULL, filename.buf); html(" "); } if (free_ref) free((char *)ref); + strbuf_release(&filename); } static int print_tag(struct refinfo *ref) diff --git a/ui-repolist.c b/ui-repolist.c index 76fe71a9a4bfca98c8340deba3b8ee68cb69b9c3..47ca997883923222c4797ed1c20b59348abe2058 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -33,7 +33,7 @@ } static int get_repo_modtime(const struct cgit_repo *repo, time_t *mtime) { - char *path; + struct strbuf path = STRBUF_INIT; struct stat s; struct cgit_repo *r = (struct cgit_repo *)repo; @@ -41,32 +41,36 @@ if (repo->mtime != -1) { *mtime = repo->mtime; return 1; } - path = fmt("%s/%s", repo->path, ctx.cfg.agefile); - if (stat(path, &s) == 0) { - *mtime = read_agefile(path); + strbuf_addf(&path, "%s/%s", repo->path, ctx.cfg.agefile); + if (stat(path.buf, &s) == 0) { + *mtime = read_agefile(path.buf); if (*mtime) { r->mtime = *mtime; - return 1; + goto end; } } - path = fmt("%s/refs/heads/%s", repo->path, repo->defbranch ? - repo->defbranch : "master"); - if (stat(path, &s) == 0) { + strbuf_reset(&path); + strbuf_addf(&path, "%s/refs/heads/%s", repo->path, + repo->defbranch ? repo->defbranch : "master"); + if (stat(path.buf, &s) == 0) { *mtime = s.st_mtime; r->mtime = *mtime; - return 1; + goto end; } - path = fmt("%s/%s", repo->path, "packed-refs"); - if (stat(path, &s) == 0) { + strbuf_reset(&path); + strbuf_addf(&path, "%s/%s", repo->path, "packed-refs"); + if (stat(path.buf, &s) == 0) { *mtime = s.st_mtime; r->mtime = *mtime; - return 1; + goto end; } *mtime = 0; r->mtime = *mtime; +end: + strbuf_release(&path); return (r->mtime != 0); } diff --git a/ui-shared.c b/ui-shared.c index b93b77aee4f5718ad74e36a3bc3d4d3a3ed9f65e..519eef7102c4d61a960620bc587917027656da3d 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -62,7 +62,7 @@ if (!ctx.env.server_name) return NULL; if (!ctx.env.server_port || atoi(ctx.env.server_port) == 80) return ctx.env.server_name; - return xstrdup(fmt("%s:%s", ctx.env.server_name, ctx.env.server_port)); + return fmtalloc("%s:%s", ctx.env.server_name, ctx.env.server_port); } const char *cgit_rooturl() @@ -75,31 +75,30 @@ } char *cgit_repourl(const char *reponame) { - if (ctx.cfg.virtual_root) { - return fmt("%s%s/", ctx.cfg.virtual_root, reponame); - } else { - return fmt("?r=%s", reponame); - } + if (ctx.cfg.virtual_root) + return fmtalloc("%s%s/", ctx.cfg.virtual_root, reponame); + else + return fmtalloc("?r=%s", reponame); } char *cgit_fileurl(const char *reponame, const char *pagename, const char *filename, const char *query) { - char *tmp; + struct strbuf sb = STRBUF_INIT; char *delim; if (ctx.cfg.virtual_root) { - tmp = fmt("%s%s/%s/%s", ctx.cfg.virtual_root, reponame, - pagename, (filename ? filename:"")); + strbuf_addf(&sb, "%s%s/%s/%s", ctx.cfg.virtual_root, reponame, + pagename, (filename ? filename:"")); delim = "?"; } else { - tmp = fmt("?url=%s/%s/%s", reponame, pagename, - (filename ? filename : "")); + strbuf_addf(&sb, "?url=%s/%s/%s", reponame, pagename, + (filename ? filename : "")); delim = "&"; } if (query) - tmp = fmt("%s%s%s", tmp, delim, query); - return tmp; + strbuf_addf(&sb, "%s%s", delim, query); + return strbuf_detach(&sb, NULL); } char *cgit_pageurl(const char *reponame, const char *pagename, @@ -548,21 +547,21 @@ if (class) htmlf("class='%s' ", class); html("href='"); if (item) { - html_attr(fmt(item->util, rev)); + html_attrf(item->util, rev); } else if (ctx.repo->module_link) { dir = strrchr(path, '/'); if (dir) dir++; else dir = path; - html_attr(fmt(ctx.repo->module_link, dir, rev)); + html_attrf(ctx.repo->module_link, dir, rev); } else { html("#"); } html("'>"); html_txt(path); html("</a>"); - html_txt(fmt(" @ %.7s", rev)); + html_txtf(" @ %.7s", rev); if (item && tail) path[len - 1] = tail; } @@ -678,12 +677,16 @@ html_attr(ctx->cfg.favicon); html("'/>\n"); } if (host && ctx->repo && ctx->qry.head) { + struct strbuf sb = STRBUF_INIT; + strbuf_addf(&sb, "h=%s", ctx->qry.head); + html("<link rel='alternate' title='Atom feed' href='"); html(cgit_httpscheme()); html_attr(cgit_hosturl()); html_attr(cgit_fileurl(ctx->repo->url, "atom", ctx->qry.vpath, - fmt("h=%s", ctx->qry.head))); + sb.buf)); html("' type='application/atom+xml'/>\n"); + strbuf_release(&sb); } if (ctx->cfg.head_include) html_include(ctx->cfg.head_include); @@ -725,13 +728,14 @@ void cgit_add_hidden_formfields(int incl_head, int incl_search, const char *page) { - char *url; + if (!ctx.cfg.virtual_root) { + struct strbuf url = STRBUF_INIT; - if (!ctx.cfg.virtual_root) { - url = fmt("%s/%s", ctx.qry.repo, page); + strbuf_addf(&url, "%s/%s", ctx.qry.repo, page); if (ctx.qry.vpath) - url = fmt("%s/%s", url, ctx.qry.vpath); - html_hidden("url", url); + strbuf_addf(&url, "/%s", ctx.qry.vpath); + html_hidden("url", url.buf); + strbuf_release(&url); } if (incl_head && ctx.qry.head && ctx.repo->defbranch && @@ -926,20 +930,23 @@ void cgit_print_snapshot_links(const char *repo, const char *head, const char *hex, int snapshots) { const struct cgit_snapshot_format* f; - char *prefix; - char *filename; + struct strbuf filename = STRBUF_INIT; + size_t prefixlen; unsigned char sha1[20]; if (get_sha1(fmt("refs/tags/%s", hex), sha1) == 0 && (hex[0] == 'v' || hex[0] == 'V') && isdigit(hex[1])) hex++; - prefix = xstrdup(fmt("%s-%s", cgit_repobasename(repo), hex)); + strbuf_addf(&filename, "%s-%s", cgit_repobasename(repo), hex); + prefixlen = filename.len; for (f = cgit_snapshot_formats; f->suffix; f++) { if (!(snapshots & f->bit)) continue; - filename = fmt("%s%s", prefix, f->suffix); - cgit_snapshot_link(filename, NULL, NULL, NULL, NULL, filename); + strbuf_setlen(&filename, prefixlen); + strbuf_addstr(&filename, f->suffix); + cgit_snapshot_link(filename.buf, NULL, NULL, NULL, NULL, + filename.buf); html("<br/>"); } - free(prefix); + strbuf_release(&filename); } diff --git a/ui-snapshot.c b/ui-snapshot.c index a47884e85556ff75d24364f67f441994d339ca7b..8e76977cf42d737cc8500395262a1e4ff94d27e7 100644 --- a/ui-snapshot.c +++ b/ui-snapshot.c @@ -15,14 +15,33 @@ static int write_archive_type(const char *format, const char *hex, const char *prefix) { struct argv_array argv = ARGV_ARRAY_INIT; + const char **nargv; + int result; argv_array_push(&argv, "snapshot"); argv_array_push(&argv, format); if (prefix) { + struct strbuf buf = STRBUF_INIT; + strbuf_addstr(&buf, prefix); + strbuf_addch(&buf, '/'); argv_array_push(&argv, "--prefix"); - argv_array_push(&argv, fmt("%s/", prefix)); + argv_array_push(&argv, buf.buf); + strbuf_release(&buf); } argv_array_push(&argv, hex); - return write_archive(argv.argc, argv.argv, NULL, 1, NULL, 0); + /* + * Now we need to copy the pointers to arguments into a new + * structure because write_archive will rearrange its arguments + * which may result in duplicated/missing entries causing leaks + * or double-frees in argv_array_clear. + */ + nargv = xmalloc(sizeof(char *) * (argv.argc + 1)); + /* argv_array guarantees a trailing NULL entry. */ + memcpy(nargv, argv.argv, sizeof(char *) * (argv.argc + 1)); + + result = write_archive(argv.argc, nargv, NULL, 1, NULL, 0); + argv_array_clear(&argv); + free(nargv); + return result; } static int write_tar_archive(const char *hex, const char *prefix) @@ -129,29 +148,36 @@ const struct cgit_snapshot_format *format) { const char *reponame; unsigned char sha1[20]; - char *snapshot; + struct strbuf snapshot = STRBUF_INIT; + int result = 1; - snapshot = xstrdup(filename); - snapshot[strlen(snapshot) - strlen(format->suffix)] = '\0'; + strbuf_addstr(&snapshot, filename); + strbuf_setlen(&snapshot, snapshot.len - strlen(format->suffix)); - if (get_sha1(snapshot, sha1) == 0) - return snapshot; + if (get_sha1(snapshot.buf, sha1) == 0) + goto out; reponame = cgit_repobasename(url); - if (prefixcmp(snapshot, reponame) == 0) { - snapshot += strlen(reponame); - while (snapshot && (*snapshot == '-' || *snapshot == '_')) - snapshot++; + if (prefixcmp(snapshot.buf, reponame) == 0) { + const char *new_start = snapshot.buf; + new_start += strlen(reponame); + while (new_start && (*new_start == '-' || *new_start == '_')) + new_start++; + strbuf_splice(&snapshot, 0, new_start - snapshot.buf, "", 0); } - if (get_sha1(snapshot, sha1) == 0) - return snapshot; + if (get_sha1(snapshot.buf, sha1) == 0) + goto out; - snapshot = fmt("v%s", snapshot); - if (get_sha1(snapshot, sha1) == 0) - return snapshot; + strbuf_insert(&snapshot, 0, "v", 1); + if (get_sha1(snapshot.buf, sha1) == 0) + goto out; - return NULL; + result = 0; + strbuf_release(&snapshot); + +out: + return result ? strbuf_detach(&snapshot, NULL) : NULL; } __attribute__((format (printf, 1, 2))) diff --git a/ui-summary.c b/ui-summary.c index bd123ef4b81c78dd265fc0116f1ba8e00871b044..f965b320b4033a01aca83adac5f3b6f91dfeab7a 100644 --- a/ui-summary.c +++ b/ui-summary.c @@ -17,6 +17,7 @@ static void print_url(char *base, char *suffix) { int columns = 3; + struct strbuf basebuf = STRBUF_INIT; if (ctx.repo->enable_log_filecount) columns++; @@ -25,13 +26,16 @@ columns++; if (!base || !*base) return; - if (suffix && *suffix) - base = fmt("%s/%s", base, suffix); + if (suffix && *suffix) { + strbuf_addf(&basebuf, "%s/%s", base, suffix); + base = basebuf.buf; + } htmlf("<tr><td colspan='%d'><a href='", columns); html_url_path(base); html("'>"); html_txt(base); html("</a></td></tr>\n"); + strbuf_release(&basebuf); } static void print_urls(char *txt, char *suffix) @@ -112,8 +116,8 @@ } /* Prepend repo path to relative readme path unless tracked. */ if (!ref && *ctx.repo->readme != '/') - ctx.repo->readme = xstrdup(fmt("%s/%s", ctx.repo->path, - ctx.repo->readme)); + ctx.repo->readme = fmtalloc("%s/%s", ctx.repo->path, + ctx.repo->readme); /* If a subpath is specified for the about page, make it relative * to the directory containing the configured readme. diff --git a/ui-tag.c b/ui-tag.c index 397e15b83638531eae7adf931f043c18b5032a76..aea795879dfe595f851efe4908e8d31fb2976837 100644 --- a/ui-tag.c +++ b/ui-tag.c @@ -41,6 +41,7 @@ } void cgit_print_tag(char *revname) { + struct strbuf fullref = STRBUF_INIT; unsigned char sha1[20]; struct object *obj; struct tag *tag; @@ -49,20 +50,21 @@ if (!revname) revname = ctx.qry.head; - if (get_sha1(fmt("refs/tags/%s", revname), sha1)) { + strbuf_addf(&fullref, "refs/tags/%s", revname); + if (get_sha1(fullref.buf, sha1)) { cgit_print_error("Bad tag reference: %s", revname); - return; + goto cleanup; } obj = parse_object(sha1); if (!obj) { cgit_print_error("Bad object id: %s", sha1_to_hex(sha1)); - return; + goto cleanup; } if (obj->type == OBJ_TAG) { tag = lookup_tag(sha1); if (!tag || parse_tag(tag) || !(info = cgit_parse_tag(tag))) { cgit_print_error("Bad tag object: %s", revname); - return; + goto cleanup; } html("<table class='commit-info'>\n"); htmlf("<tr><td>tag name</td><td>"); @@ -101,5 +103,7 @@ if (ctx.repo->snapshots) print_download_links(revname); html("</table>\n"); } - return; + +cleanup: + strbuf_release(&fullref); } diff --git a/ui-tree.c b/ui-tree.c index aebe145cb88585ff2c05f4f3f915c47f7a914ae1..aa5dee93c46a680d3acd62f61b801ab3f25024ab 100644 --- a/ui-tree.c +++ b/ui-tree.c @@ -129,14 +129,14 @@ void *cbdata) { struct walk_tree_context *walk_tree_ctx = cbdata; char *name; - char *fullpath; - char *class; + struct strbuf fullpath = STRBUF_INIT; + struct strbuf class = STRBUF_INIT; enum object_type type; unsigned long size = 0; name = xstrdup(pathname); - fullpath = fmt("%s%s%s", ctx.qry.path ? ctx.qry.path : "", - ctx.qry.path ? "/" : "", name); + strbuf_addf(&fullpath, "%s%s%s", ctx.qry.path ? ctx.qry.path : "", + ctx.qry.path ? "/" : "", name); if (!S_ISGITLINK(mode)) { type = sha1_object_info(sha1, &size); @@ -152,33 +152,34 @@ html(""); cgit_print_filemode(mode); html("</td><td>"); if (S_ISGITLINK(mode)) { - cgit_submodule_link("ls-mod", fullpath, sha1_to_hex(sha1)); + cgit_submodule_link("ls-mod", fullpath.buf, sha1_to_hex(sha1)); } else if (S_ISDIR(mode)) { cgit_tree_link(name, NULL, "ls-dir", ctx.qry.head, - walk_tree_ctx->curr_rev, fullpath); + walk_tree_ctx->curr_rev, fullpath.buf); } else { - class = strrchr(name, '.'); - if (class != NULL) { - class = fmt("ls-blob %s", class + 1); - } else - class = "ls-blob"; - cgit_tree_link(name, NULL, class, ctx.qry.head, - walk_tree_ctx->curr_rev, fullpath); + char *ext = strrchr(name, '.'); + strbuf_addstr(&class, "ls-blob"); + if (ext) + strbuf_addf(&class, " %s", ext + 1); + cgit_tree_link(name, NULL, class.buf, ctx.qry.head, + walk_tree_ctx->curr_rev, fullpath.buf); } htmlf("</td><td class='ls-size'>%li</td>", size); html("<td>"); cgit_log_link("log", NULL, "button", ctx.qry.head, - walk_tree_ctx->curr_rev, fullpath, 0, NULL, NULL, + walk_tree_ctx->curr_rev, fullpath.buf, 0, NULL, NULL, ctx.qry.showmsg); if (ctx.repo->max_stats) cgit_stats_link("stats", NULL, "button", ctx.qry.head, - fullpath); + fullpath.buf); if (!S_ISGITLINK(mode)) cgit_plain_link("plain", NULL, "button", ctx.qry.head, - walk_tree_ctx->curr_rev, fullpath); + walk_tree_ctx->curr_rev, fullpath.buf); html("</td></tr>\n"); free(name); + strbuf_release(&fullpath); + strbuf_release(&class); return 0; }