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; } 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 @@ end: 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 @@ void scan_projects(const char *path, const char *projectsfile, repo_config_fn fn 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 8592843..93af0ce 100644 --- a/ui-log.c +++ b/ui-log.c @@ -243,15 +243,19 @@ static void print_commit(struct commit *commit, struct rev_info *revs) 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 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern 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 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern } 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 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern ctx.qry.vpath, 0, NULL, NULL, ctx.qry.showmsg); html("\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 6b0d84b..9c86542 100644 --- a/ui-plain.c +++ b/ui-plain.c @@ -109,9 +109,9 @@ static int print_object(const unsigned char *sha1, const char *path) 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 @@ static void print_dir(const unsigned char *sha1, const char *base, fullpath); html("\n"); } + free(fullpath); } static void print_dir_entry(const unsigned char *sha1, const char *base, @@ -159,6 +160,7 @@ static void print_dir_entry(const unsigned char *sha1, const char *base, cgit_plain_link(path, NULL, NULL, ctx.qry.head, ctx.qry.sha1, fullpath); html("\n"); + free(fullpath); } static void print_dir_tail(void) diff --git a/ui-refs.c b/ui-refs.c index 7406478..3fbaad0 100644 --- a/ui-refs.c +++ b/ui-refs.c @@ -99,7 +99,7 @@ static void print_tag_header() 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 @@ static void print_tag_downloads(const struct cgit_repo *repo, const char *ref) 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 @@ static void print_tag_downloads(const struct cgit_repo *repo, const char *ref) 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 76fe71a..47ca997 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -33,7 +33,7 @@ static time_t read_agefile(char *path) 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 @@ static int get_repo_modtime(const struct cgit_repo *repo, time_t *mtime) *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 b93b77a..519eef7 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -62,7 +62,7 @@ const char *cgit_hosturl() 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 @@ const char *cgit_rooturl() 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 @@ void cgit_submodule_link(const char *class, char *path, const char *rev) 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(""); - html_txt(fmt(" @ %.7s", rev)); + html_txtf(" @ %.7s", rev); if (item && tail) path[len - 1] = tail; } @@ -678,12 +677,16 @@ void cgit_print_docstart(struct cgit_context *ctx) html("'/>\n"); } if (host && ctx->repo && ctx->qry.head) { + struct strbuf sb = STRBUF_INIT; + strbuf_addf(&sb, "h=%s", ctx->qry.head); + html("\n"); + strbuf_release(&sb); } if (ctx->cfg.head_include) html_include(ctx->cfg.head_include); @@ -725,13 +728,14 @@ static int print_branch_option(const char *refname, const unsigned char *sha1, void cgit_add_hidden_formfields(int incl_head, int incl_search, const char *page) { - char *url; - if (!ctx.cfg.virtual_root) { - url = fmt("%s/%s", ctx.qry.repo, page); + struct strbuf url = STRBUF_INIT; + + 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("
"); } - free(prefix); + strbuf_release(&filename); } diff --git a/ui-snapshot.c b/ui-snapshot.c index a47884e..8e76977 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 @@ static const char *get_ref_from_filename(const char *url, const char *filename, { 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 bd123ef..f965b32 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 @@ static void print_url(char *base, char *suffix) 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(""); html_txt(base); html("\n"); + strbuf_release(&basebuf); } static void print_urls(char *txt, char *suffix) @@ -112,8 +116,8 @@ void cgit_print_repo_readme(char *path) /* 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 397e15b..aea7958 100644 --- a/ui-tag.c +++ b/ui-tag.c @@ -41,6 +41,7 @@ static void print_download_links(char *revname) 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 @@ void cgit_print_tag(char *revname) 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("\n"); htmlf("
tag name"); @@ -101,5 +103,7 @@ void cgit_print_tag(char *revname) print_download_links(revname); html("
\n"); } - return; + +cleanup: + strbuf_release(&fullref); } diff --git a/ui-tree.c b/ui-tree.c index aebe145..aa5dee9 100644 --- a/ui-tree.c +++ b/ui-tree.c @@ -129,14 +129,14 @@ static int ls_item(const unsigned char *sha1, const char *base, int baselen, { 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 @@ static int ls_item(const unsigned char *sha1, const char *base, int baselen, cgit_print_filemode(mode); html(""); 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("%li", size); html(""); 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("\n"); free(name); + strbuf_release(&fullpath); + strbuf_release(&class); return 0; }