* [PATCH 00/19] Fixed-size buffer removal @ 2013-04-07 9:29 john 2013-04-07 9:29 ` [PATCH 01/19] Fix out-of-bounds memory accesses with virtual_root="" john ` (21 more replies) 0 siblings, 22 replies; 75+ messages in thread From: john @ 2013-04-07 9:29 UTC (permalink / raw) This series replaces use of fixed-size buffers for any user-supplied input or data from the repository with Git's struct strbuf. It is based on wip. After this series, the only remaining uses of html.c::fmt produce strings with a known bound on their length and there are no uses of snprintf. The first patch was sent before but appears to have been lost in the noise; I'm resending it as part of this series to avoid textual conflicts with later patches. While working on this, I learned about the "module-link" configuration option, which currently takes a printf format string which is passed to printf as the format string. I'd like to change this to use the strbuf_expand format so that administrators of CGit installations can't shoot themselves in the foot with invalid format strings but I'd like some feedback on how we can do that and maintain backwards compatibility. Perhaps we could introduce "submodule-link" with the new syntax and deprecate "module-link"? John Keeping (19): Fix out-of-bounds memory accesses with virtual_root="" Remove redundant calls to fmt("%s", ...) cache.c: don't use statically sized buffers for filenames html: introduce html_txtf and html_vtxtf functions Convert cgit_print_error to a variadic function scan-tree: use struct strbuf instead of static buffers ui-log.c: use a strbuf for refs ui-log.c: use a strbuf for grep arguments ui-plain.c: use struct strbuf instead of fmt() ui-refs.c: use struct strbuf instead of fixed-size buffers ui-repolist.c: use struct strbuf for repository paths ui-snapshot.c: tidy up memory management in write_archive_type ui-snapshot: use a struct strbuf instead of fixed-size buffers ui-summary.c: use struct strbuf instead of fixed-size buffers ui-tag.c: use struct strbuf for user-supplied data ui-tree.c: use struct strbuf instead of fmt() cgit.c: use struct strbuf instead of fmt() html: add html_attrf to output an attribute value from a format string ui-shared.c: use struct strbuf instead of fmt() cache.c | 56 ++++++++--------------- cgit.c | 105 +++++++++++++++++++++--------------------- cgit.h | 3 +- html.c | 41 +++++++++++++++-- html.h | 11 ++++- scan-tree.c | 145 ++++++++++++++++++++++++++++++++-------------------------- shared.c | 15 ++++++ ui-blob.c | 8 ++-- ui-commit.c | 4 +- ui-diff.c | 8 ++-- ui-log.c | 32 ++++++++----- ui-patch.c | 4 +- ui-plain.c | 10 ++-- ui-refs.c | 11 +++-- ui-repolist.c | 28 +++++++----- ui-shared.c | 66 +++++++++++++++----------- ui-shared.h | 5 +- ui-snapshot.c | 65 ++++++++++++++++---------- ui-stats.c | 5 +- ui-summary.c | 16 +++++-- ui-tag.c | 20 ++++---- ui-tree.c | 46 +++++++++---------- 22 files changed, 410 insertions(+), 294 deletions(-) -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 01/19] Fix out-of-bounds memory accesses with virtual_root="" 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john @ 2013-04-07 9:29 ` john 2013-04-07 9:29 ` [PATCH 02/19] Remove redundant calls to fmt("%s", ...) john ` (20 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:29 UTC (permalink / raw) The CGit configuration variable virtual_root is normalized so that it does not have a trailing '/' character, but it is allowed to be empty (the empty string and NULL have different meanings here) and there is code that is insufficiently cautious when checking if it ends in a '/': if (virtual_root[strlen(virtual_root) - 1] != '/') Clearly this check is redundant, but rather than simply removing it we get a slight efficiency improvement by switching the normalization so that the virtual_root variable always ends in '/'. Do this with a new "ensure_end" helper. Signed-off-by: John Keeping <john at keeping.me.uk> --- cgit.c | 11 +++-------- cgit.h | 3 ++- shared.c | 15 +++++++++++++++ ui-shared.c | 14 +++++--------- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/cgit.c b/cgit.c index ca3034c..6f75db1 100644 --- a/cgit.c +++ b/cgit.c @@ -155,9 +155,7 @@ static void config_cb(const char *name, const char *value) else if (!strcmp(name, "strict-export")) ctx.cfg.strict_export = xstrdup(value); else if (!strcmp(name, "virtual-root")) { - ctx.cfg.virtual_root = trim_end(value, '/'); - if (!ctx.cfg.virtual_root && (!strcmp(value, "/"))) - ctx.cfg.virtual_root = ""; + ctx.cfg.virtual_root = ensure_end(value, '/'); } else if (!strcmp(name, "nocache")) ctx.cfg.nocache = atoi(value); else if (!strcmp(name, "noplainemail")) @@ -833,11 +831,8 @@ int main(int argc, const char **argv) * that virtual-root equals SCRIPT_NAME, minus any possibly * trailing slashes. */ - if (!ctx.cfg.virtual_root && ctx.cfg.script_name) { - ctx.cfg.virtual_root = trim_end(ctx.cfg.script_name, '/'); - if (!ctx.cfg.virtual_root) - ctx.cfg.virtual_root = ""; - } + if (!ctx.cfg.virtual_root && ctx.cfg.script_name) + ctx.cfg.virtual_root = ensure_end(ctx.cfg.script_name, '/'); /* If no url parameter is specified on the querystring, lets * use PATH_INFO as url. This allows cgit to work with virtual diff --git a/cgit.h b/cgit.h index 081f669..fc3fc6f 100644 --- a/cgit.h +++ b/cgit.h @@ -190,7 +190,7 @@ struct cgit_config { char *script_name; char *section; char *repository_sort; - char *virtual_root; + char *virtual_root; /* Always ends with '/'. */ char *strict_export; int cache_size; int cache_dynamic_ttl; @@ -300,6 +300,7 @@ extern int chk_positive(int result, char *msg); extern int chk_non_negative(int result, char *msg); extern char *trim_end(const char *str, char c); +extern char *ensure_end(const char *str, char c); extern char *strlpart(char *txt, int maxlen); extern char *strrpart(char *txt, int maxlen); diff --git a/shared.c b/shared.c index cc06930..1fa9c99 100644 --- a/shared.c +++ b/shared.c @@ -115,6 +115,21 @@ char *trim_end(const char *str, char c) return xstrndup(str, len); } +char *ensure_end(const char *str, char c) +{ + size_t len = strlen(str); + char *result; + + if (len && str[len - 1] == c) + return xstrndup(str, len); + + result = xmalloc(len + 2); + memcpy(result, str, len); + result[len] = '/'; + result[len + 1] = '\0'; + return result; +} + char *strlpart(char *txt, int maxlen) { char *result; diff --git a/ui-shared.c b/ui-shared.c index 945d560..c1f3c20 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -57,7 +57,7 @@ const char *cgit_hosturl() const char *cgit_rooturl() { if (ctx.cfg.virtual_root) - return fmt("%s/", ctx.cfg.virtual_root); + return ctx.cfg.virtual_root; else return ctx.cfg.script_name; } @@ -65,7 +65,7 @@ const char *cgit_rooturl() char *cgit_repourl(const char *reponame) { if (ctx.cfg.virtual_root) { - return fmt("%s/%s/", ctx.cfg.virtual_root, reponame); + return fmt("%s%s/", ctx.cfg.virtual_root, reponame); } else { return fmt("?r=%s", reponame); } @@ -78,7 +78,7 @@ char *cgit_fileurl(const char *reponame, const char *pagename, char *delim; if (ctx.cfg.virtual_root) { - tmp = fmt("%s/%s/%s/%s", ctx.cfg.virtual_root, reponame, + tmp = fmt("%s%s/%s/%s", ctx.cfg.virtual_root, reponame, pagename, (filename ? filename:"")); delim = "?"; } else { @@ -126,11 +126,9 @@ static void site_url(const char *page, const char *search, const char *sort, int { char *delim = "?"; - if (ctx.cfg.virtual_root) { + if (ctx.cfg.virtual_root) html_attr(ctx.cfg.virtual_root); - if (ctx.cfg.virtual_root[strlen(ctx.cfg.virtual_root) - 1] != '/') - html("/"); - } else + else html(ctx.cfg.script_name); if (page) { @@ -201,8 +199,6 @@ static char *repolink(const char *title, const char *class, const char *page, html(" href='"); if (ctx.cfg.virtual_root) { html_url_path(ctx.cfg.virtual_root); - if (ctx.cfg.virtual_root[strlen(ctx.cfg.virtual_root) - 1] != '/') - html("/"); html_url_path(ctx.repo->url); if (ctx.repo->url[strlen(ctx.repo->url) - 1] != '/') html("/"); -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 02/19] Remove redundant calls to fmt("%s", ...) 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john 2013-04-07 9:29 ` [PATCH 01/19] Fix out-of-bounds memory accesses with virtual_root="" john @ 2013-04-07 9:29 ` john 2013-04-07 11:05 ` Jason 2013-04-07 9:29 ` [PATCH 03/19] cache.c: don't use statically sized buffers for filenames john ` (19 subsequent siblings) 21 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 9:29 UTC (permalink / raw) After this change there is one remaining call 'fmt("%s", delim)' in ui-shared.c but is needed as delim is stack allocated and so cannot be returned from the function. Signed-off-by: John Keeping <john at keeping.me.uk> --- scan-tree.c | 4 ++-- ui-plain.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scan-tree.c b/scan-tree.c index 0d3e0ad..05caba5 100644 --- a/scan-tree.c +++ b/scan-tree.c @@ -96,9 +96,9 @@ static void add_repo(const char *base, const char *path, repo_config_fn fn) return; if (base == path) - rel = xstrdup(fmt("%s", path)); + rel = xstrdup(path); else - rel = xstrdup(fmt("%s", path + strlen(base) + 1)); + rel = xstrdup(path + strlen(base) + 1); if (!strcmp(rel + strlen(rel) - 5, "/.git")) rel[strlen(rel) - 5] = '\0'; diff --git a/ui-plain.c b/ui-plain.c index 4397a59..4c6cbb7 100644 --- a/ui-plain.c +++ b/ui-plain.c @@ -95,7 +95,7 @@ static int print_object(const unsigned char *sha1, const char *path) else ctx.page.mimetype = "text/plain"; } - ctx.page.filename = fmt("%s", path); + ctx.page.filename = "%s"; ctx.page.size = size; ctx.page.etag = sha1_to_hex(sha1); cgit_print_http_headers(&ctx); -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 02/19] Remove redundant calls to fmt("%s", ...) 2013-04-07 9:29 ` [PATCH 02/19] Remove redundant calls to fmt("%s", ...) john @ 2013-04-07 11:05 ` Jason 2013-04-07 11:25 ` john 0 siblings, 1 reply; 75+ messages in thread From: Jason @ 2013-04-07 11:05 UTC (permalink / raw) On Sun, Apr 7, 2013 at 11:29 AM, John Keeping <john at keeping.me.uk> wrote: > - ctx.page.filename = fmt("%s", path); > + ctx.page.filename = "%s"; These aren't exactly equivalent. Is there something I'm missing here (I'm going through this patch set linearly.)? ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 02/19] Remove redundant calls to fmt("%s", ...) 2013-04-07 11:05 ` Jason @ 2013-04-07 11:25 ` john 2013-04-07 11:30 ` Jason 0 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 11:25 UTC (permalink / raw) On Sun, Apr 07, 2013 at 01:05:40PM +0200, Jason A. Donenfeld wrote: > On Sun, Apr 7, 2013 at 11:29 AM, John Keeping <john at keeping.me.uk> wrote: > > - ctx.page.filename = fmt("%s", path); > > + ctx.page.filename = "%s"; > > These aren't exactly equivalent. Is there something I'm missing here > (I'm going through this patch set linearly.)? That I was on autopilot at this stage and didn't check this thoroughly enough :-( Clearly it should be: + ctx.page.filename = path; ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 02/19] Remove redundant calls to fmt("%s", ...) 2013-04-07 11:25 ` john @ 2013-04-07 11:30 ` Jason 0 siblings, 0 replies; 75+ messages in thread From: Jason @ 2013-04-07 11:30 UTC (permalink / raw) On Sun, Apr 7, 2013 at 1:25 PM, John Keeping <john at keeping.me.uk> wrote: > That I was on autopilot at this stage and didn't check this thoroughly > enough :-( That's what I'm here for; no problemo. > > Clearly it should be: > > + ctx.page.filename = path; -- Jason A. Donenfeld Deep Space Explorer fr: +33 6 51 90 82 66 us: +1 513 476 1200 www.jasondonenfeld.com ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 03/19] cache.c: don't use statically sized buffers for filenames 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john 2013-04-07 9:29 ` [PATCH 01/19] Fix out-of-bounds memory accesses with virtual_root="" john 2013-04-07 9:29 ` [PATCH 02/19] Remove redundant calls to fmt("%s", ...) john @ 2013-04-07 9:29 ` john 2013-04-07 11:11 ` Jason 2013-04-07 9:29 ` [PATCH 04/19] html: introduce html_txtf and html_vtxtf functions john ` (18 subsequent siblings) 21 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 9:29 UTC (permalink / raw) Instead use "struct strbuf" from Git to remove the limit on file path length. Signed-off-by: John Keeping <john at keeping.me.uk> --- cache.c | 56 +++++++++++++++++++------------------------------------- 1 file changed, 19 insertions(+), 37 deletions(-) diff --git a/cache.c b/cache.c index 3127fc2..4399ad8 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,23 @@ 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); + if (filename.buf[filename.len - 1] != '/') + strbuf_addch(&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 +372,12 @@ 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; 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,27 @@ 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); + if (fullname.buf[fullname.len - 1] != '/') + strbuf_addch(&fullname, '/'); while ((ent = readdir(dir)) != NULL) { if (strlen(ent->d_name) != 8) continue; - strcpy(name, ent->d_name); + 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; } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 03/19] cache.c: don't use statically sized buffers for filenames 2013-04-07 9:29 ` [PATCH 03/19] cache.c: don't use statically sized buffers for filenames john @ 2013-04-07 11:11 ` Jason 2013-04-07 11:30 ` john 0 siblings, 1 reply; 75+ messages in thread From: Jason @ 2013-04-07 11:11 UTC (permalink / raw) On Sun, Apr 7, 2013 at 11:29 AM, John Keeping <john at keeping.me.uk> wrote: > + if (filename.buf[filename.len - 1] != '/') > + strbuf_addch(&filename, '/'); > + if (fullname.buf[fullname.len - 1] != '/') > + strbuf_addch(&fullname, '/'); Seems like it might be handy to have an ensure_end for strbuf (and perhaps eventually depreciate the char* ensure_end if that code is refactored to use strbuf as well). ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 03/19] cache.c: don't use statically sized buffers for filenames 2013-04-07 11:11 ` Jason @ 2013-04-07 11:30 ` john 0 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 11:30 UTC (permalink / raw) On Sun, Apr 07, 2013 at 01:11:55PM +0200, Jason A. Donenfeld wrote: > On Sun, Apr 7, 2013 at 11:29 AM, John Keeping <john at keeping.me.uk> wrote: > > + if (filename.buf[filename.len - 1] != '/') > > + strbuf_addch(&filename, '/'); > > > + if (fullname.buf[fullname.len - 1] != '/') > > + strbuf_addch(&fullname, '/'); > > > Seems like it might be handy to have an ensure_end for strbuf (and > perhaps eventually depreciate the char* ensure_end if that code is > refactored to use strbuf as well). Good idea. I'll work that into a reroll along with anything else that comes up. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 04/19] html: introduce html_txtf and html_vtxtf functions 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (2 preceding siblings ...) 2013-04-07 9:29 ` [PATCH 03/19] cache.c: don't use statically sized buffers for filenames john @ 2013-04-07 9:29 ` john 2013-04-07 9:29 ` [PATCH 05/19] Convert cgit_print_error to a variadic function john ` (17 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:29 UTC (permalink / raw) These takes a printf style format string like htmlf but escapes the resulting string. The html_vtxtf variant takes a va_list whereas html_txtf is variadic. Signed-off-by: John Keeping <john at keeping.me.uk> --- html.c | 28 +++++++++++++++++++++++++--- html.h | 8 +++++++- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/html.c b/html.c index 8c45ba6..f193786 100644 --- a/html.c +++ b/html.c @@ -76,13 +76,35 @@ void html(const char *txt) void htmlf(const char *format, ...) { - static char buf[65536]; va_list args; + struct strbuf buf = STRBUF_INIT; va_start(args, format); - vsnprintf(buf, sizeof(buf), format, args); + strbuf_vaddf(&buf, format, args); va_end(args); - html(buf); + html(buf.buf); + strbuf_release(&buf); +} + +void html_txtf(const char *format, ...) +{ + va_list args; + + va_start(args, format); + html_vtxtf(format, args); + va_end(args); +} + +void html_vtxtf(const char *format, va_list ap) +{ + va_list cp; + struct strbuf buf = STRBUF_INIT; + + va_copy(cp, ap); + strbuf_vaddf(&buf, format, cp); + va_end(cp); + html_txt(buf.buf); + strbuf_release(&buf); } void html_status(int code, const char *msg, int more_headers) diff --git a/html.h b/html.h index bb36f37..6886a46 100644 --- a/html.h +++ b/html.h @@ -1,7 +1,7 @@ #ifndef HTML_H #define HTML_H -#include <stddef.h> +#include "cgit.h" extern void html_raw(const char *txt, size_t size); extern void html(const char *txt); @@ -9,6 +9,12 @@ extern void html(const char *txt); __attribute__((format (printf,1,2))) extern void htmlf(const char *format,...); +__attribute__((format (printf,1,2))) +extern void html_txtf(const char *format,...); + +__attribute__((format (printf,1,0))) +extern void html_vtxtf(const char *format, va_list ap); + extern void html_status(int code, const char *msg, int more_headers); extern void html_txt(const char *txt); extern void html_ntxt(int len, const char *txt); -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 05/19] Convert cgit_print_error to a variadic function 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (3 preceding siblings ...) 2013-04-07 9:29 ` [PATCH 04/19] html: introduce html_txtf and html_vtxtf functions john @ 2013-04-07 9:29 ` john 2013-04-07 9:29 ` [PATCH 06/19] scan-tree: use struct strbuf instead of static buffers john ` (16 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:29 UTC (permalink / raw) This removes many uses of "fmt" which uses a fixed size static pool of fixed size buffers. Instead of relying on these, we now pass around argument lists for as long as possible before using a strbuf to render content of an arbitrary size. Signed-off-by: John Keeping <john at keeping.me.uk> --- cgit.c | 18 ++++++++---------- ui-blob.c | 8 ++++---- ui-commit.c | 4 ++-- ui-diff.c | 8 ++++---- ui-patch.c | 4 ++-- ui-shared.c | 15 +++++++++++++-- ui-shared.h | 5 ++++- ui-snapshot.c | 16 ++++++++++------ ui-stats.c | 5 ++--- ui-tag.c | 6 +++--- ui-tree.c | 13 +++++-------- 11 files changed, 57 insertions(+), 45 deletions(-) diff --git a/cgit.c b/cgit.c index 6f75db1..bf42023 100644 --- a/cgit.c +++ b/cgit.c @@ -459,7 +459,6 @@ static char *guess_defbranch(void) static int prepare_repo_cmd(struct cgit_context *ctx) { - char *tmp; unsigned char sha1[20]; int nongit = 0; int rc; @@ -470,14 +469,13 @@ static int prepare_repo_cmd(struct cgit_context *ctx) rc = errno; ctx->page.title = fmt("%s - %s", ctx->cfg.root_title, "config error"); - tmp = fmt("Failed to open %s: %s", - ctx->repo->name, - rc ? strerror(rc) : "Not a valid git repository"); ctx->repo = NULL; cgit_print_http_headers(ctx); cgit_print_docstart(ctx); cgit_print_pageheader(ctx); - cgit_print_error(tmp); + cgit_print_error("Failed to open %s: %s", + ctx->repo->name, + rc ? strerror(rc) : "Not a valid git repository"); cgit_print_docend(); return 1; } @@ -501,14 +499,14 @@ static int prepare_repo_cmd(struct cgit_context *ctx) } if (get_sha1(ctx->qry.head, sha1)) { - tmp = xstrdup(ctx->qry.head); + char *tmp = xstrdup(ctx->qry.head); ctx->qry.head = ctx->repo->defbranch; ctx->page.status = 404; ctx->page.statusmsg = "Not found"; cgit_print_http_headers(ctx); cgit_print_docstart(ctx); cgit_print_pageheader(ctx); - cgit_print_error(fmt("Invalid branch: %s", tmp)); + cgit_print_error("Invalid branch: %s", tmp); cgit_print_docend(); return 1; } @@ -550,7 +548,7 @@ static void process_request(void *cbdata) cgit_print_http_headers(ctx); cgit_print_docstart(ctx); cgit_print_pageheader(ctx); - cgit_print_error(fmt("No repository selected")); + cgit_print_error("No repository selected"); cgit_print_docend(); return; } @@ -862,7 +860,7 @@ int main(int argc, const char **argv) err = cache_process(ctx.cfg.cache_size, ctx.cfg.cache_root, ctx.qry.raw, ttl, process_request, &ctx); if (err) - cgit_print_error(fmt("Error processing page: %s (%d)", - strerror(err), err)); + cgit_print_error("Error processing page: %s (%d)", + strerror(err), err); return err; } diff --git a/ui-blob.c b/ui-blob.c index 0c6c215..1525115 100644 --- a/ui-blob.c +++ b/ui-blob.c @@ -94,12 +94,12 @@ void cgit_print_blob(const char *hex, char *path, const char *head) if (hex) { if (get_sha1_hex(hex, sha1)) { - cgit_print_error(fmt("Bad hex value: %s", hex)); + cgit_print_error("Bad hex value: %s", hex); return; } } else { if (get_sha1(head, sha1)) { - cgit_print_error(fmt("Bad ref: %s", head)); + cgit_print_error("Bad ref: %s", head); return; } } @@ -113,13 +113,13 @@ void cgit_print_blob(const char *hex, char *path, const char *head) } if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", hex)); + cgit_print_error("Bad object name: %s", hex); return; } buf = read_sha1_file(sha1, &type, &size); if (!buf) { - cgit_print_error(fmt("Error reading object %s", hex)); + cgit_print_error("Error reading object %s", hex); return; } diff --git a/ui-commit.c b/ui-commit.c index 105ef13..a392432 100644 --- a/ui-commit.c +++ b/ui-commit.c @@ -27,12 +27,12 @@ void cgit_print_commit(char *hex, const char *prefix) hex = ctx.qry.head; if (get_sha1(hex, sha1)) { - cgit_print_error(fmt("Bad object id: %s", hex)); + cgit_print_error("Bad object id: %s", hex); return; } commit = lookup_commit_reference(sha1); if (!commit) { - cgit_print_error(fmt("Bad commit reference: %s", hex)); + cgit_print_error("Bad commit reference: %s", hex); return; } info = cgit_parse_commit(commit); diff --git a/ui-diff.c b/ui-diff.c index 1115518..8b38209 100644 --- a/ui-diff.c +++ b/ui-diff.c @@ -369,12 +369,12 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, get_sha1(new_rev, new_rev_sha1); type = sha1_object_info(new_rev_sha1, &size); if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", new_rev)); + cgit_print_error("Bad object name: %s", new_rev); return; } commit = lookup_commit_reference(new_rev_sha1); if (!commit || parse_commit(commit)) { - cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(new_rev_sha1))); + cgit_print_error("Bad commit: %s", sha1_to_hex(new_rev_sha1)); return; } @@ -388,12 +388,12 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, if (!is_null_sha1(old_rev_sha1)) { type = sha1_object_info(old_rev_sha1, &size); if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", sha1_to_hex(old_rev_sha1))); + cgit_print_error("Bad object name: %s", sha1_to_hex(old_rev_sha1)); return; } commit2 = lookup_commit_reference(old_rev_sha1); if (!commit2 || parse_commit(commit2)) { - cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(old_rev_sha1))); + cgit_print_error("Bad commit: %s", sha1_to_hex(old_rev_sha1)); return; } } diff --git a/ui-patch.c b/ui-patch.c index 66def3c..fbb92cc 100644 --- a/ui-patch.c +++ b/ui-patch.c @@ -94,12 +94,12 @@ void cgit_print_patch(char *hex, const char *prefix) hex = ctx.qry.head; if (get_sha1(hex, sha1)) { - cgit_print_error(fmt("Bad object id: %s", hex)); + cgit_print_error("Bad object id: %s", hex); return; } commit = lookup_commit_reference(sha1); if (!commit) { - cgit_print_error(fmt("Bad commit reference: %s", hex)); + cgit_print_error("Bad commit reference: %s", hex); return; } info = cgit_parse_commit(commit); diff --git a/ui-shared.c b/ui-shared.c index c1f3c20..b93b77a 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -28,10 +28,21 @@ static char *http_date(time_t t) tm->tm_hour, tm->tm_min, tm->tm_sec); } -void cgit_print_error(const char *msg) +void cgit_print_error(const char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + cgit_vprint_error(fmt, ap); + va_end(ap); +} + +void cgit_vprint_error(const char *fmt, va_list ap) +{ + va_list cp; html("<div class='error'>"); - html_txt(msg); + va_copy(cp, ap); + html_vtxtf(fmt, cp); + va_end(cp); html("</div>\n"); } diff --git a/ui-shared.h b/ui-shared.h index 5f8b629..5987e77 100644 --- a/ui-shared.h +++ b/ui-shared.h @@ -52,7 +52,10 @@ extern void cgit_object_link(struct object *obj); extern void cgit_submodule_link(const char *class, char *path, const char *rev); -extern void cgit_print_error(const char *msg); +__attribute__((format (printf,1,2))) +extern void cgit_print_error(const char *fmt, ...); +__attribute__((format (printf,1,0))) +extern void cgit_vprint_error(const char *fmt, va_list ap); extern void cgit_print_date(time_t secs, const char *format, int local_time); extern void cgit_print_age(time_t t, time_t max_relative, const char *format); extern void cgit_print_http_headers(struct cgit_context *ctx); diff --git a/ui-snapshot.c b/ui-snapshot.c index 9be5dbe..a47884e 100644 --- a/ui-snapshot.c +++ b/ui-snapshot.c @@ -100,11 +100,11 @@ static int make_snapshot(const struct cgit_snapshot_format *format, unsigned char sha1[20]; if (get_sha1(hex, sha1)) { - cgit_print_error(fmt("Bad object id: %s", hex)); + cgit_print_error("Bad object id: %s", hex); return 1; } if (!lookup_commit_reference(sha1)) { - cgit_print_error(fmt("Not a commit reference: %s", hex)); + cgit_print_error("Not a commit reference: %s", hex); return 1; } ctx.page.mimetype = xstrdup(format->mimetype); @@ -154,13 +154,18 @@ static const char *get_ref_from_filename(const char *url, const char *filename, return NULL; } -static void show_error(char *msg) +__attribute__((format (printf, 1, 2))) +static void show_error(char *fmt, ...) { + va_list ap; + ctx.page.mimetype = "text/html"; cgit_print_http_headers(&ctx); cgit_print_docstart(&ctx); cgit_print_pageheader(&ctx); - cgit_print_error(msg); + va_start(ap, fmt); + cgit_vprint_error(fmt, ap); + va_end(ap); cgit_print_docend(); } @@ -177,8 +182,7 @@ void cgit_print_snapshot(const char *head, const char *hex, f = get_format(filename); if (!f) { - show_error(xstrdup(fmt("Unsupported snapshot format: %s", - filename))); + show_error("Unsupported snapshot format: %s", filename); return; } diff --git a/ui-stats.c b/ui-stats.c index 52e9b91..28b794f 100644 --- a/ui-stats.c +++ b/ui-stats.c @@ -374,12 +374,11 @@ void cgit_show_stats(struct cgit_context *ctx) i = cgit_find_stats_period(code, &period); if (!i) { - cgit_print_error(fmt("Unknown statistics type: %c", code[0])); + cgit_print_error("Unknown statistics type: %c", code[0]); return; } if (i > ctx->repo->max_stats) { - cgit_print_error(fmt("Statistics type disabled: %s", - period->name)); + cgit_print_error("Statistics type disabled: %s", period->name); return; } authors = collect_stats(ctx, period); diff --git a/ui-tag.c b/ui-tag.c index 5a22696..397e15b 100644 --- a/ui-tag.c +++ b/ui-tag.c @@ -50,18 +50,18 @@ void cgit_print_tag(char *revname) revname = ctx.qry.head; if (get_sha1(fmt("refs/tags/%s", revname), sha1)) { - cgit_print_error(fmt("Bad tag reference: %s", revname)); + cgit_print_error("Bad tag reference: %s", revname); return; } obj = parse_object(sha1); if (!obj) { - cgit_print_error(fmt("Bad object id: %s", sha1_to_hex(sha1))); + cgit_print_error("Bad object id: %s", sha1_to_hex(sha1)); return; } if (obj->type == OBJ_TAG) { tag = lookup_tag(sha1); if (!tag || parse_tag(tag) || !(info = cgit_parse_tag(tag))) { - cgit_print_error(fmt("Bad tag object: %s", revname)); + cgit_print_error("Bad tag object: %s", revname); return; } html("<table class='commit-info'>\n"); diff --git a/ui-tree.c b/ui-tree.c index d713553..aebe145 100644 --- a/ui-tree.c +++ b/ui-tree.c @@ -95,15 +95,13 @@ static void print_object(const unsigned char *sha1, char *path, const char *base type = sha1_object_info(sha1, &size); if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", - sha1_to_hex(sha1))); + cgit_print_error("Bad object name: %s", sha1_to_hex(sha1)); return; } buf = read_sha1_file(sha1, &type, &size); if (!buf) { - cgit_print_error(fmt("Error reading object %s", - sha1_to_hex(sha1))); + cgit_print_error("Error reading object %s", sha1_to_hex(sha1)); return; } @@ -209,8 +207,7 @@ static void ls_tree(const unsigned char *sha1, char *path, struct walk_tree_cont tree = parse_tree_indirect(sha1); if (!tree) { - cgit_print_error(fmt("Not a tree object: %s", - sha1_to_hex(sha1))); + cgit_print_error("Not a tree object: %s", sha1_to_hex(sha1)); return; } @@ -273,12 +270,12 @@ void cgit_print_tree(const char *rev, char *path) rev = ctx.qry.head; if (get_sha1(rev, sha1)) { - cgit_print_error(fmt("Invalid revision name: %s", rev)); + cgit_print_error("Invalid revision name: %s", rev); return; } commit = lookup_commit_reference(sha1); if (!commit || parse_commit(commit)) { - cgit_print_error(fmt("Invalid commit reference: %s", rev)); + cgit_print_error("Invalid commit reference: %s", rev); return; } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 06/19] scan-tree: use struct strbuf instead of static buffers 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (4 preceding siblings ...) 2013-04-07 9:29 ` [PATCH 05/19] Convert cgit_print_error to a variadic function john @ 2013-04-07 9:29 ` john 2013-04-07 11:29 ` Jason 2013-04-07 9:29 ` [PATCH 07/19] ui-log.c: use a strbuf for refs john ` (15 subsequent siblings) 21 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 9:29 UTC (permalink / raw) 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 '/'). Signed-off-by: John Keeping <john at keeping.me.uk> --- scan-tree.c | 145 +++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 79 insertions(+), 66 deletions(-) diff --git a/scan-tree.c b/scan-tree.c index 05caba5..e696af9 100644 --- a/scan-tree.c +++ b/scan-tree.c @@ -12,19 +12,14 @@ #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; - 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); @@ -33,8 +28,9 @@ static int is_git_dir(const char *path) if (!S_ISDIR(st.st_mode)) return 0; - 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); @@ -75,47 +71,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 (ctx.cfg.strict_export) { + strbuf_addstr(path, ctx.cfg.strict_export); + if(stat(path->buf, &st)) + return; + strbuf_setlen(path, pathlen); + } - if (!stat(fmt("%s/noweb", path), &st)) + 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 +135,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 +170,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 +190,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 +215,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 +234,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,13 +244,12 @@ 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", -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 06/19] scan-tree: use struct strbuf instead of static buffers 2013-04-07 9:29 ` [PATCH 06/19] scan-tree: use struct strbuf instead of static buffers john @ 2013-04-07 11:29 ` Jason 2013-04-07 11:33 ` john 0 siblings, 1 reply; 75+ messages in thread From: Jason @ 2013-04-07 11:29 UTC (permalink / raw) On Sun, Apr 7, 2013 at 11:29 AM, John Keeping <john at keeping.me.uk> wrote: > static int is_git_dir(const char *path) > { > struct stat st; > - static char buf[MAX_PATH]; > + struct strbuf pathbuf = STRBUF_INIT; > > - 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); strbuf_release is never called. Does this leak? Are there other cases of this elsewhere? ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 06/19] scan-tree: use struct strbuf instead of static buffers 2013-04-07 11:29 ` Jason @ 2013-04-07 11:33 ` john 0 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 11:33 UTC (permalink / raw) On Sun, Apr 07, 2013 at 01:29:47PM +0200, Jason A. Donenfeld wrote: > On Sun, Apr 7, 2013 at 11:29 AM, John Keeping <john at keeping.me.uk> wrote: > > static int is_git_dir(const char *path) > > { > > struct stat st; > > - static char buf[MAX_PATH]; > > + struct strbuf pathbuf = STRBUF_INIT; > > > > - 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); > > strbuf_release is never called. Does this leak? Yes. > Are there other cases of this elsewhere? I thought I cleaned up everywhere, hopefully this is the only one I missed but I'll go through and check again. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 07/19] ui-log.c: use a strbuf for refs 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (5 preceding siblings ...) 2013-04-07 9:29 ` [PATCH 06/19] scan-tree: use struct strbuf instead of static buffers john @ 2013-04-07 9:29 ` john 2013-04-07 9:29 ` [PATCH 08/19] ui-log.c: use a strbuf for grep arguments john ` (14 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:29 UTC (permalink / raw) This avoids using a fixed-size buffer in fmt() for a user-supplied ref string. Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-log.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/ui-log.c b/ui-log.c index d75d7bf..7c3308c 100644 --- a/ui-log.c +++ b/ui-log.c @@ -244,15 +244,18 @@ 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; return ref; } @@ -285,6 +288,7 @@ 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; + int must_free_tip = 0; char *arg; /* First argv is NULL */ @@ -292,7 +296,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern 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) { @@ -430,4 +434,8 @@ 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("</td></tr>\n"); } + + /* If we allocated tip then it is safe to cast away const. */ + if (must_free_tip) + free((char*) tip); } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 08/19] ui-log.c: use a strbuf for grep arguments 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (6 preceding siblings ...) 2013-04-07 9:29 ` [PATCH 07/19] ui-log.c: use a strbuf for refs john @ 2013-04-07 9:29 ` john 2013-04-07 9:30 ` [PATCH 09/19] ui-plain.c: use struct strbuf instead of fmt() john ` (13 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:29 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-log.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ui-log.c b/ui-log.c index 7c3308c..44f6004 100644 --- a/ui-log.c +++ b/ui-log.c @@ -289,7 +289,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern struct vector vec = VECTOR_INIT(char *); int i, columns = commit_graph ? 4 : 3; int must_free_tip = 0; - char *arg; + struct strbuf argbuf = STRBUF_INIT; /* First argv is NULL */ vector_push(&vec, NULL, 0); @@ -303,10 +303,11 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *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 @@ -341,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); } @@ -438,4 +439,5 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern /* If we allocated tip then it is safe to cast away const. */ if (must_free_tip) free((char*) tip); + strbuf_release(&argbuf); } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 09/19] ui-plain.c: use struct strbuf instead of fmt() 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (7 preceding siblings ...) 2013-04-07 9:29 ` [PATCH 08/19] ui-log.c: use a strbuf for grep arguments john @ 2013-04-07 9:30 ` john 2013-04-07 9:30 ` [PATCH 10/19] ui-refs.c: use struct strbuf instead of fixed-size buffers john ` (12 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:30 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-plain.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ui-plain.c b/ui-plain.c index 4c6cbb7..9793804 100644 --- a/ui-plain.c +++ b/ui-plain.c @@ -107,10 +107,12 @@ static int print_object(const unsigned char *sha1, const char *path) static char *buildpath(const char *base, int baselen, const char *path) { + struct strbuf buf = STRBUF_INIT; if (path[0]) - return fmt("%.*s%s/", baselen, base, path); + strbuf_addf(&buf, "%.*s%s/", baselen, base, path); else - return fmt("%.*s/", baselen, base); + strbuf_addf(&buf, "%.*s/", baselen, base); + return strbuf_detach(&buf, NULL); } static void print_dir(const unsigned char *sha1, const char *base, @@ -141,6 +143,7 @@ static void print_dir(const unsigned char *sha1, const char *base, fullpath); html("</li>\n"); } + free(fullpath); } static void print_dir_entry(const unsigned char *sha1, const char *base, @@ -158,6 +161,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("</li>\n"); + free(fullpath); } static void print_dir_tail(void) -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 10/19] ui-refs.c: use struct strbuf instead of fixed-size buffers 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (8 preceding siblings ...) 2013-04-07 9:30 ` [PATCH 09/19] ui-plain.c: use struct strbuf instead of fmt() john @ 2013-04-07 9:30 ` john 2013-04-07 13:08 ` Jason 2013-04-07 9:30 ` [PATCH 11/19] ui-repolist.c: use struct strbuf for repository paths john ` (11 subsequent siblings) 21 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 9:30 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-refs.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/ui-refs.c b/ui-refs.c index 5bebed1..399c157 100644 --- a/ui-refs.c +++ b/ui-refs.c @@ -102,7 +102,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; @@ -114,7 +114,9 @@ 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)); + struct strbuf buf = STRBUF_INIT; + strbuf_addf(&buf, "%s-%s", basename, ref); + ref = strbuf_detach(&buf, NULL); free_ref = 1; } } @@ -122,13 +124,14 @@ 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_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) { -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 10/19] ui-refs.c: use struct strbuf instead of fixed-size buffers 2013-04-07 9:30 ` [PATCH 10/19] ui-refs.c: use struct strbuf instead of fixed-size buffers john @ 2013-04-07 13:08 ` Jason 0 siblings, 0 replies; 75+ messages in thread From: Jason @ 2013-04-07 13:08 UTC (permalink / raw) On Sun, Apr 7, 2013 at 11:30 AM, John Keeping <john at keeping.me.uk> wrote: > @@ -122,13 +124,14 @@ 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_addf(&filename, "%s%s", ref, f->suffix); > + cgit_snapshot_link(filename.buf, NULL, NULL, NULL, NULL, filename.buf); > html(" "); > } You need to call strbuf_reset inside that loop. Otherwise the filenames will be concatenated for each type. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 11/19] ui-repolist.c: use struct strbuf for repository paths 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (9 preceding siblings ...) 2013-04-07 9:30 ` [PATCH 10/19] ui-refs.c: use struct strbuf instead of fixed-size buffers john @ 2013-04-07 9:30 ` john 2013-04-07 9:30 ` [PATCH 12/19] ui-snapshot.c: tidy up memory management in write_archive_type john ` (10 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:30 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-repolist.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) 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); } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 12/19] ui-snapshot.c: tidy up memory management in write_archive_type 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (10 preceding siblings ...) 2013-04-07 9:30 ` [PATCH 11/19] ui-repolist.c: use struct strbuf for repository paths john @ 2013-04-07 9:30 ` john 2013-04-07 9:30 ` [PATCH 13/19] ui-snapshot: use a struct strbuf instead of fixed-size buffers john ` (9 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:30 UTC (permalink / raw) - Use a strbuf instead of a fixed-size buffer - Free the argv_array when we're done with it Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-snapshot.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ui-snapshot.c b/ui-snapshot.c index a47884e..2d47676 100644 --- a/ui-snapshot.c +++ b/ui-snapshot.c @@ -15,14 +15,21 @@ static int write_archive_type(const char *format, const char *hex, const char *prefix) { struct argv_array argv = ARGV_ARRAY_INIT; + 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); + result = write_archive(argv.argc, argv.argv, NULL, 1, NULL, 0); + argv_array_clear(&argv); + return result; } static int write_tar_archive(const char *hex, const char *prefix) -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 13/19] ui-snapshot: use a struct strbuf instead of fixed-size buffers 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (11 preceding siblings ...) 2013-04-07 9:30 ` [PATCH 12/19] ui-snapshot.c: tidy up memory management in write_archive_type john @ 2013-04-07 9:30 ` john 2013-04-07 13:25 ` Jason 2013-04-07 13:33 ` Jason 2013-04-07 9:30 ` [PATCH 14/19] ui-summary.c: use " john ` (8 subsequent siblings) 21 siblings, 2 replies; 75+ messages in thread From: john @ 2013-04-07 9:30 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-snapshot.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/ui-snapshot.c b/ui-snapshot.c index 2d47676..b5dab0f 100644 --- a/ui-snapshot.c +++ b/ui-snapshot.c @@ -136,29 +136,37 @@ 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_reset(&snapshot); + strbuf_addf(&snapshot, "v%s", snapshot.buf); + 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))) -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 13/19] ui-snapshot: use a struct strbuf instead of fixed-size buffers 2013-04-07 9:30 ` [PATCH 13/19] ui-snapshot: use a struct strbuf instead of fixed-size buffers john @ 2013-04-07 13:25 ` Jason 2013-04-07 13:37 ` john 2013-04-07 13:33 ` Jason 1 sibling, 1 reply; 75+ messages in thread From: Jason @ 2013-04-07 13:25 UTC (permalink / raw) On Sun, Apr 7, 2013 at 11:30 AM, John Keeping <john at keeping.me.uk> wrote: > + 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); > } Something funky is happening here. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 13/19] ui-snapshot: use a struct strbuf instead of fixed-size buffers 2013-04-07 13:25 ` Jason @ 2013-04-07 13:37 ` john 2013-04-07 13:39 ` Jason 0 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 13:37 UTC (permalink / raw) On Sun, Apr 07, 2013 at 03:25:36PM +0200, Jason A. Donenfeld wrote: > On Sun, Apr 7, 2013 at 11:30 AM, John Keeping <john at keeping.me.uk> wrote: > > + 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); > > } > > Something funky is happening here. Care to be more specific? I think this behaviour is the same as the code it replaces - we're removing the leading reponame and any following '-' or '_' characters from the start of snapshot: - 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); } The strbuf_splice ends up just shifting the start of snapshot left by "new_start - snapshot.buf" bytes, which is precisely the same as changing the pointer to point at new_start (except that we can continue to perform strbuf operations). Of course, the following code is completely bogus :-( + strbuf_reset(&snapshot); + strbuf_addf(&snapshot, "v%s", snapshot.buf); I'll fix that. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 13/19] ui-snapshot: use a struct strbuf instead of fixed-size buffers 2013-04-07 13:37 ` john @ 2013-04-07 13:39 ` Jason 0 siblings, 0 replies; 75+ messages in thread From: Jason @ 2013-04-07 13:39 UTC (permalink / raw) On Sun, Apr 7, 2013 at 3:37 PM, John Keeping <john at keeping.me.uk> wrote: > Care to be more specific? I think this behaviour is the same as the > code it replaces. You're right -- I sent the follow-up email to say "whoops, that's not the code that's bad. It's this code instead, below." ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 13/19] ui-snapshot: use a struct strbuf instead of fixed-size buffers 2013-04-07 9:30 ` [PATCH 13/19] ui-snapshot: use a struct strbuf instead of fixed-size buffers john 2013-04-07 13:25 ` Jason @ 2013-04-07 13:33 ` Jason 1 sibling, 0 replies; 75+ messages in thread From: Jason @ 2013-04-07 13:33 UTC (permalink / raw) On Sun, Apr 7, 2013 at 11:30 AM, John Keeping <john at keeping.me.uk> wrote: > + strbuf_reset(&snapshot); > + strbuf_addf(&snapshot, "v%s", snapshot.buf); Scrap the previous message. Here's where the bug is. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 14/19] ui-summary.c: use struct strbuf instead of fixed-size buffers 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (12 preceding siblings ...) 2013-04-07 9:30 ` [PATCH 13/19] ui-snapshot: use a struct strbuf instead of fixed-size buffers john @ 2013-04-07 9:30 ` john 2013-04-07 12:20 ` Jason 2013-04-07 9:30 ` [PATCH 15/19] ui-tag.c: use struct strbuf for user-supplied data john ` (7 subsequent siblings) 21 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 9:30 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-summary.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/ui-summary.c b/ui-summary.c index bd123ef..91f4061 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("<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) @@ -111,9 +115,11 @@ 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)); + if (!ref && *ctx.repo->readme != '/') { + struct strbuf buf = STRBUF_INIT; + strbuf_addf(&buf, "%s/%s", ctx.repo->path, ctx.repo->readme); + ctx.repo->readme = strbuf_detach(&buf, NULL); + } /* If a subpath is specified for the about page, make it relative * to the directory containing the configured readme. -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 14/19] ui-summary.c: use struct strbuf instead of fixed-size buffers 2013-04-07 9:30 ` [PATCH 14/19] ui-summary.c: use " john @ 2013-04-07 12:20 ` Jason 2013-04-07 12:36 ` john 0 siblings, 1 reply; 75+ messages in thread From: Jason @ 2013-04-07 12:20 UTC (permalink / raw) On Sun, Apr 7, 2013 at 11:30 AM, John Keeping <john at keeping.me.uk> wrote: > /* 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)); > + if (!ref && *ctx.repo->readme != '/') { > + struct strbuf buf = STRBUF_INIT; > + strbuf_addf(&buf, "%s/%s", ctx.repo->path, ctx.repo->readme); > + ctx.repo->readme = strbuf_detach(&buf, NULL); > + } I do in fact see the merits of using strbuf and the purpose of this patch set, but OTOH, this little chunk here illustrates the downsides -- the extra verbosity this adds is really a bummer. Perhaps this could be moved into a new fmtalloc helper function, if this pattern is common? ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 14/19] ui-summary.c: use struct strbuf instead of fixed-size buffers 2013-04-07 12:20 ` Jason @ 2013-04-07 12:36 ` john 2013-04-07 12:41 ` Jason 0 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 12:36 UTC (permalink / raw) On Sun, Apr 07, 2013 at 02:20:26PM +0200, Jason A. Donenfeld wrote: > On Sun, Apr 7, 2013 at 11:30 AM, John Keeping <john at keeping.me.uk> wrote: > > /* 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)); > > + if (!ref && *ctx.repo->readme != '/') { > > + struct strbuf buf = STRBUF_INIT; > > + strbuf_addf(&buf, "%s/%s", ctx.repo->path, ctx.repo->readme); > > + ctx.repo->readme = strbuf_detach(&buf, NULL); > > + } > > I do in fact see the merits of using strbuf and the purpose of this > patch set, but OTOH, this little chunk here illustrates the downsides > -- the extra verbosity this adds is really a bummer. Perhaps this > could be moved into a new fmtalloc helper function, if this pattern is > common? That could be sensible. I didn't do that initially because a lot of the uses are just: allocate format use free and there is no real saving from a helper in that case, but in case where we format an then detach the helper could be useful. I'll do that in the reroll. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 14/19] ui-summary.c: use struct strbuf instead of fixed-size buffers 2013-04-07 12:36 ` john @ 2013-04-07 12:41 ` Jason 2013-04-07 12:43 ` Jason 0 siblings, 1 reply; 75+ messages in thread From: Jason @ 2013-04-07 12:41 UTC (permalink / raw) On Sun, Apr 7, 2013 at 2:36 PM, John Keeping <john at keeping.me.uk> wrote: > free Any opinions on using gcc's __attribute__((cleanup))? It could make things a bit cleaner, but it's something of an awful gcc hack. #define strbuf_auto struct strbuf __attribute__((cleanup(strbuf_release)) ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 14/19] ui-summary.c: use struct strbuf instead of fixed-size buffers 2013-04-07 12:41 ` Jason @ 2013-04-07 12:43 ` Jason 0 siblings, 0 replies; 75+ messages in thread From: Jason @ 2013-04-07 12:43 UTC (permalink / raw) On Sun, Apr 7, 2013 at 2:41 PM, Jason A. Donenfeld <Jason at zx2c4.com> wrote: > Any opinions on using gcc's __attribute__((cleanup))? It could make > things a bit cleaner, but it's something of an awful gcc hack. > > #define strbuf_auto struct strbuf __attribute__((cleanup(strbuf_release)) -fexceptions... nevermind. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 15/19] ui-tag.c: use struct strbuf for user-supplied data 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (13 preceding siblings ...) 2013-04-07 9:30 ` [PATCH 14/19] ui-summary.c: use " john @ 2013-04-07 9:30 ` john 2013-04-07 9:30 ` [PATCH 16/19] ui-tree.c: use struct strbuf instead of fmt() john ` (6 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:30 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-tag.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) 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("<table class='commit-info'>\n"); htmlf("<tr><td>tag name</td><td>"); @@ -101,5 +103,7 @@ void cgit_print_tag(char *revname) print_download_links(revname); html("</table>\n"); } - return; + +cleanup: + strbuf_release(&fullref); } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 16/19] ui-tree.c: use struct strbuf instead of fmt() 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (14 preceding siblings ...) 2013-04-07 9:30 ` [PATCH 15/19] ui-tag.c: use struct strbuf for user-supplied data john @ 2013-04-07 9:30 ` john 2013-04-07 9:30 ` [PATCH 17/19] cgit.c: " john ` (5 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:30 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-tree.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) 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("</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; } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 17/19] cgit.c: use struct strbuf instead of fmt() 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (15 preceding siblings ...) 2013-04-07 9:30 ` [PATCH 16/19] ui-tree.c: use struct strbuf instead of fmt() john @ 2013-04-07 9:30 ` john 2013-04-07 9:30 ` [PATCH 18/19] html: add html_attrf to output an attribute value from a format string john ` (4 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:30 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- cgit.c | 76 +++++++++++++++++++++++++++++++++++------------------------------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/cgit.c b/cgit.c index bf42023..c226aaa 100644 --- a/cgit.c +++ b/cgit.c @@ -460,6 +460,7 @@ static char *guess_defbranch(void) static int prepare_repo_cmd(struct cgit_context *ctx) { unsigned char sha1[20]; + struct strbuf title = STRBUF_INIT; int nongit = 0; int rc; @@ -467,8 +468,9 @@ static int prepare_repo_cmd(struct cgit_context *ctx) setup_git_directory_gently(&nongit); if (nongit) { rc = errno; - ctx->page.title = fmt("%s - %s", ctx->cfg.root_title, - "config error"); + strbuf_addf(&title, "%s - %s", ctx->cfg.root_title, + "config error"); + ctx->page.title = strbuf_detach(&title, NULL); ctx->repo = NULL; cgit_print_http_headers(ctx); cgit_print_docstart(ctx); @@ -479,7 +481,8 @@ 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); + strbuf_addf(&title, "%s - %s", ctx->repo->name, ctx->repo->desc); + ctx->page.title = strbuf_detach(&title, NULL); if (!ctx->repo->defbranch) ctx->repo->defbranch = guess_defbranch(); @@ -577,21 +580,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 +637,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 +659,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 +682,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 +816,7 @@ static int calc_ttl() int main(int argc, const char **argv) { const char *path; - char *qry; + struct strbuf qry = STRBUF_INIT; int err, ttl; prepare_context(&ctx); @@ -843,9 +847,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); + strbuf_addf(&qry, "%s?%s", path, ctx.qry.raw); + free(ctx.qry.raw); + ctx.qry.raw = strbuf_detach(&qry, NULL); } else ctx.qry.raw = xstrdup(ctx.qry.url); cgit_parse_url(ctx.qry.url); -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 18/19] html: add html_attrf to output an attribute value from a format string 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (16 preceding siblings ...) 2013-04-07 9:30 ` [PATCH 17/19] cgit.c: " john @ 2013-04-07 9:30 ` john 2013-04-07 9:30 ` [PATCH 19/19] ui-shared.c: use struct strbuf instead of fmt() john ` (3 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 9:30 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- html.c | 13 +++++++++++++ html.h | 3 +++ 2 files changed, 16 insertions(+) diff --git a/html.c b/html.c index f193786..fe4ae7e 100644 --- a/html.c +++ b/html.c @@ -158,6 +158,19 @@ void html_ntxt(int len, const char *txt) html("..."); } +void html_attrf(const char *fmt, ...) +{ + va_list ap; + struct strbuf sb = STRBUF_INIT; + + va_start(ap, fmt); + strbuf_vaddf(&sb, fmt, ap); + va_end(ap); + + html_attr(sb.buf); + strbuf_release(&sb); +} + void html_attr(const char *txt) { const char *t = txt; diff --git a/html.h b/html.h index 6886a46..be3b311 100644 --- a/html.h +++ b/html.h @@ -15,6 +15,9 @@ extern void html_txtf(const char *format,...); __attribute__((format (printf,1,0))) extern void html_vtxtf(const char *format, va_list ap); +__attribute__((format (printf,1,2))) +extern void html_attrf(const char *format,...); + extern void html_status(int code, const char *msg, int more_headers); extern void html_txt(const char *txt); extern void html_ntxt(int len, const char *txt); -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 19/19] ui-shared.c: use struct strbuf instead of fmt() 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (17 preceding siblings ...) 2013-04-07 9:30 ` [PATCH 18/19] html: add html_attrf to output an attribute value from a format string john @ 2013-04-07 9:30 ` john 2013-04-07 12:37 ` Jason 2013-04-07 13:08 ` [PATCH 00/19] Fixed-size buffer removal Jason ` (2 subsequent siblings) 21 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 9:30 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-shared.c | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/ui-shared.c b/ui-shared.c index b93b77a..b155684 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -56,13 +56,15 @@ const char *cgit_httpscheme() const char *cgit_hosturl() { + struct strbuf sb = STRBUF_INIT; if (ctx.env.http_host) return ctx.env.http_host; 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)); + strbuf_addf(&sb, "%s:%s", ctx.env.server_name, ctx.env.server_port); + return strbuf_detach(&sb, NULL); } const char *cgit_rooturl() @@ -75,31 +77,32 @@ 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); - } + struct strbuf sb = STRBUF_INIT; + if (ctx.cfg.virtual_root) + strbuf_addf(&sb, "%s%s/", ctx.cfg.virtual_root, reponame); + else + strbuf_addf(&sb, "?r=%s", reponame); + return strbuf_detach(&sb, NULL); } 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 +551,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("</a>"); - html_txt(fmt(" @ %.7s", rev)); + html_txtf(" @ %.7s", rev); if (item && tail) path[len - 1] = tail; } @@ -678,12 +681,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("<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); -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 19/19] ui-shared.c: use struct strbuf instead of fmt() 2013-04-07 9:30 ` [PATCH 19/19] ui-shared.c: use struct strbuf instead of fmt() john @ 2013-04-07 12:37 ` Jason 2013-04-07 12:44 ` john 2013-04-07 12:49 ` cgit 0 siblings, 2 replies; 75+ messages in thread From: Jason @ 2013-04-07 12:37 UTC (permalink / raw) On Sun, Apr 7, 2013 at 11:30 AM, John Keeping <john at keeping.me.uk> wrote: > const char *cgit_hosturl() > { > + struct strbuf sb = STRBUF_INIT; > if (ctx.env.http_host) > return ctx.env.http_host; > 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)); > + strbuf_addf(&sb, "%s:%s", ctx.env.server_name, ctx.env.server_port); > + return strbuf_detach(&sb, NULL); > } Constness issues, since we're going to want to perhaps deallocate this at some point? ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 19/19] ui-shared.c: use struct strbuf instead of fmt() 2013-04-07 12:37 ` Jason @ 2013-04-07 12:44 ` john 2013-04-07 12:49 ` cgit 1 sibling, 0 replies; 75+ messages in thread From: john @ 2013-04-07 12:44 UTC (permalink / raw) On Sun, Apr 07, 2013 at 02:37:09PM +0200, Jason A. Donenfeld wrote: > On Sun, Apr 7, 2013 at 11:30 AM, John Keeping <john at keeping.me.uk> wrote: > > const char *cgit_hosturl() > > { > > + struct strbuf sb = STRBUF_INIT; > > if (ctx.env.http_host) > > return ctx.env.http_host; > > 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)); > > + strbuf_addf(&sb, "%s:%s", ctx.env.server_name, ctx.env.server_port); > > + return strbuf_detach(&sb, NULL); > > } > > Constness issues, since we're going to want to perhaps deallocate this > at some point? I'd prefer to punt that until we fix the leak, since we'll need to do a bit more than just fix the const as a couple of paths return values that we either need to xstrdup or not free. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 19/19] ui-shared.c: use struct strbuf instead of fmt() 2013-04-07 12:37 ` Jason 2013-04-07 12:44 ` john @ 2013-04-07 12:49 ` cgit 1 sibling, 0 replies; 75+ messages in thread From: cgit @ 2013-04-07 12:49 UTC (permalink / raw) On Sun, Apr 07, 2013 at 02:37:09PM +0200, Jason A. Donenfeld wrote: > On Sun, Apr 7, 2013 at 11:30 AM, John Keeping <john at keeping.me.uk> wrote: > > const char *cgit_hosturl() > > { > > + struct strbuf sb = STRBUF_INIT; > > if (ctx.env.http_host) > > return ctx.env.http_host; > > 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)); > > + strbuf_addf(&sb, "%s:%s", ctx.env.server_name, ctx.env.server_port); > > + return strbuf_detach(&sb, NULL); > > } > > Constness issues, since we're going to want to perhaps deallocate this > at some point? cgit_hosturl() currently returns both statically and dynamically allocated strings (depending on the environment), so it needs to be rewritten/replaced if we want to free() this later anyway. The const return value isn't the real issue here. > > _______________________________________________ > cgit mailing list > cgit at hjemli.net > http://hjemli.net/mailman/listinfo/cgit ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 00/19] Fixed-size buffer removal 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (18 preceding siblings ...) 2013-04-07 9:30 ` [PATCH 19/19] ui-shared.c: use struct strbuf instead of fmt() john @ 2013-04-07 13:08 ` Jason 2013-04-07 13:14 ` john 2013-04-07 14:26 ` [PATCH v2 00/22] " john 2013-04-08 14:23 ` [PATCH 00/19] Fixed-size buffer removal Jason 21 siblings, 1 reply; 75+ messages in thread From: Jason @ 2013-04-07 13:08 UTC (permalink / raw) On Sun, Apr 7, 2013 at 11:29 AM, John Keeping <john at keeping.me.uk> wrote: > After this series, the only remaining uses of html.c::fmt produce > strings with a known bound on their length and there are no uses of > snprintf. What about the fmt in cgit_print_snapshot_links? ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 00/19] Fixed-size buffer removal 2013-04-07 13:08 ` [PATCH 00/19] Fixed-size buffer removal Jason @ 2013-04-07 13:14 ` john 2013-04-08 15:31 ` Jason 0 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 13:14 UTC (permalink / raw) On Sun, Apr 07, 2013 at 03:08:53PM +0200, Jason A. Donenfeld wrote: > On Sun, Apr 7, 2013 at 11:29 AM, John Keeping <john at keeping.me.uk> wrote: > > After this series, the only remaining uses of html.c::fmt produce > > strings with a known bound on their length and there are no uses of > > snprintf. > > What about the fmt in cgit_print_snapshot_links? And cgit_add_hidden_form_fields. I'll include all of those in the reroll. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 00/19] Fixed-size buffer removal 2013-04-07 13:14 ` john @ 2013-04-08 15:31 ` Jason 2013-04-08 17:38 ` john 0 siblings, 1 reply; 75+ messages in thread From: Jason @ 2013-04-08 15:31 UTC (permalink / raw) On Sun, Apr 7, 2013 at 3:14 PM, John Keeping <john at keeping.me.uk> wrote: > On Sun, Apr 07, 2013 at 03:08:53PM +0200, Jason A. Donenfeld wrote: >> On Sun, Apr 7, 2013 at 11:29 AM, John Keeping <john at keeping.me.uk> wrote: >> > After this series, the only remaining uses of html.c::fmt produce >> > strings with a known bound on their length and there are no uses of >> > snprintf. >> >> What about the fmt in cgit_print_snapshot_links? > > And cgit_add_hidden_form_fields. I'll include all of those in the > reroll. replace_tabs in ui-ssdiff.c ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 00/19] Fixed-size buffer removal 2013-04-08 15:31 ` Jason @ 2013-04-08 17:38 ` john 2013-04-08 18:28 ` Jason 0 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-08 17:38 UTC (permalink / raw) On Mon, Apr 08, 2013 at 05:31:47PM +0200, Jason A. Donenfeld wrote: > On Sun, Apr 7, 2013 at 3:14 PM, John Keeping <john at keeping.me.uk> wrote: > > On Sun, Apr 07, 2013 at 03:08:53PM +0200, Jason A. Donenfeld wrote: > >> On Sun, Apr 7, 2013 at 11:29 AM, John Keeping <john at keeping.me.uk> wrote: > >> > After this series, the only remaining uses of html.c::fmt produce > >> > strings with a known bound on their length and there are no uses of > >> > snprintf. > >> > >> What about the fmt in cgit_print_snapshot_links? > > > > And cgit_add_hidden_form_fields. I'll include all of those in the > > reroll. > > replace_tabs in ui-ssdiff.c Yeah, this version basically handles the output of "git grep 'fmt('" and "git grep 'sn?printf'". I intend to do another pass looking for strcat, strcpy, etc. but I'm not sure when I'll get round to that. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 00/19] Fixed-size buffer removal 2013-04-08 17:38 ` john @ 2013-04-08 18:28 ` Jason 0 siblings, 0 replies; 75+ messages in thread From: Jason @ 2013-04-08 18:28 UTC (permalink / raw) On Mon, Apr 8, 2013 at 7:38 PM, John Keeping <john at keeping.me.uk> wrote: > I intend to do another pass looking for strcat, strcpy, etc. but I'm not > sure when I'll get round to that. No problem. I may get to it before you do. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 00/22] Fixed-size buffer removal 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (19 preceding siblings ...) 2013-04-07 13:08 ` [PATCH 00/19] Fixed-size buffer removal Jason @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 01/22] Fix out-of-bounds memory accesses with virtual_root="" john ` (21 more replies) 2013-04-08 14:23 ` [PATCH 00/19] Fixed-size buffer removal Jason 21 siblings, 22 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) This version addresses all of Jason's comments from v1, plus a couple of other issues I found while doing a more thorough review myself. Changes to individual patches are described against each of them but there are three new patches, which should be self-explanatory: 2/22: Mark char* fields in struct cgit_page as const 4/22: html.c: add fmtalloc helper 5/22: shared.c: add strbuf_ensure_end I've also learned my lesson about my ability to get things right first time and how I shouldn't rely on the test suite, so I have spent some time clicking around the UI and everything looks good to me. John Keeping (22): Fix out-of-bounds memory accesses with virtual_root="" Mark char* fields in struct cgit_page as const Remove redundant calls to fmt("%s", ...) html.c: add fmtalloc helper shared.c: add strbuf_ensure_end cache.c: don't use statically sized buffers for filenames html: introduce html_txtf and html_vtxtf functions Convert cgit_print_error to a variadic function scan-tree: use struct strbuf instead of static buffers ui-log.c: use a strbuf for refs ui-log.c: use a strbuf for grep arguments ui-plain.c: use struct strbuf instead of fmt() ui-refs.c: use struct strbuf instead of fixed-size buffers ui-repolist.c: use struct strbuf for repository paths ui-snapshot.c: tidy up memory management in write_archive_type ui-snapshot: use a struct strbuf instead of fixed-size buffers ui-summary.c: use struct strbuf instead of fixed-size buffers ui-tag.c: use struct strbuf for user-supplied data ui-tree.c: use struct strbuf instead of fmt() cgit.c: use struct strbuf instead of fmt() html: add html_attrf to output an attribute value from a format string ui-shared.c: use struct strbuf instead of fmt() cache.c | 57 ++++++++------------- cgit.c | 101 +++++++++++++++++------------------- cgit.h | 20 +++++--- html.c | 53 +++++++++++++++++-- html.h | 11 +++- scan-tree.c | 160 ++++++++++++++++++++++++++++++++-------------------------- shared.c | 21 ++++++++ ui-blob.c | 8 +-- ui-commit.c | 4 +- ui-diff.c | 8 +-- ui-log.c | 33 ++++++++---- ui-patch.c | 4 +- ui-plain.c | 11 ++-- ui-refs.c | 10 ++-- ui-repolist.c | 28 +++++----- ui-shared.c | 89 ++++++++++++++++++-------------- ui-shared.h | 5 +- ui-snapshot.c | 76 +++++++++++++++++++--------- ui-stats.c | 5 +- ui-summary.c | 12 +++-- ui-tag.c | 20 +++++--- ui-tree.c | 46 ++++++++--------- 22 files changed, 466 insertions(+), 316 deletions(-) -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 01/22] Fix out-of-bounds memory accesses with virtual_root="" 2013-04-07 14:26 ` [PATCH v2 00/22] " john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 02/22] Mark char* fields in struct cgit_page as const john ` (20 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) The CGit configuration variable virtual_root is normalized so that it does not have a trailing '/' character, but it is allowed to be empty (the empty string and NULL have different meanings here) and there is code that is insufficiently cautious when checking if it ends in a '/': if (virtual_root[strlen(virtual_root) - 1] != '/') Clearly this check is redundant, but rather than simply removing it we get a slight efficiency improvement by switching the normalization so that the virtual_root variable always ends in '/'. Do this with a new "ensure_end" helper. Signed-off-by: John Keeping <john at keeping.me.uk> --- cgit.c | 11 +++-------- cgit.h | 3 ++- shared.c | 15 +++++++++++++++ ui-shared.c | 14 +++++--------- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/cgit.c b/cgit.c index ca3034c..6f75db1 100644 --- a/cgit.c +++ b/cgit.c @@ -155,9 +155,7 @@ static void config_cb(const char *name, const char *value) else if (!strcmp(name, "strict-export")) ctx.cfg.strict_export = xstrdup(value); else if (!strcmp(name, "virtual-root")) { - ctx.cfg.virtual_root = trim_end(value, '/'); - if (!ctx.cfg.virtual_root && (!strcmp(value, "/"))) - ctx.cfg.virtual_root = ""; + ctx.cfg.virtual_root = ensure_end(value, '/'); } else if (!strcmp(name, "nocache")) ctx.cfg.nocache = atoi(value); else if (!strcmp(name, "noplainemail")) @@ -833,11 +831,8 @@ int main(int argc, const char **argv) * that virtual-root equals SCRIPT_NAME, minus any possibly * trailing slashes. */ - if (!ctx.cfg.virtual_root && ctx.cfg.script_name) { - ctx.cfg.virtual_root = trim_end(ctx.cfg.script_name, '/'); - if (!ctx.cfg.virtual_root) - ctx.cfg.virtual_root = ""; - } + if (!ctx.cfg.virtual_root && ctx.cfg.script_name) + ctx.cfg.virtual_root = ensure_end(ctx.cfg.script_name, '/'); /* If no url parameter is specified on the querystring, lets * use PATH_INFO as url. This allows cgit to work with virtual diff --git a/cgit.h b/cgit.h index 081f669..fc3fc6f 100644 --- a/cgit.h +++ b/cgit.h @@ -190,7 +190,7 @@ struct cgit_config { char *script_name; char *section; char *repository_sort; - char *virtual_root; + char *virtual_root; /* Always ends with '/'. */ char *strict_export; int cache_size; int cache_dynamic_ttl; @@ -300,6 +300,7 @@ extern int chk_positive(int result, char *msg); extern int chk_non_negative(int result, char *msg); extern char *trim_end(const char *str, char c); +extern char *ensure_end(const char *str, char c); extern char *strlpart(char *txt, int maxlen); extern char *strrpart(char *txt, int maxlen); diff --git a/shared.c b/shared.c index cc06930..1fa9c99 100644 --- a/shared.c +++ b/shared.c @@ -115,6 +115,21 @@ char *trim_end(const char *str, char c) return xstrndup(str, len); } +char *ensure_end(const char *str, char c) +{ + size_t len = strlen(str); + char *result; + + if (len && str[len - 1] == c) + return xstrndup(str, len); + + result = xmalloc(len + 2); + memcpy(result, str, len); + result[len] = '/'; + result[len + 1] = '\0'; + return result; +} + char *strlpart(char *txt, int maxlen) { char *result; diff --git a/ui-shared.c b/ui-shared.c index 945d560..c1f3c20 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -57,7 +57,7 @@ const char *cgit_hosturl() const char *cgit_rooturl() { if (ctx.cfg.virtual_root) - return fmt("%s/", ctx.cfg.virtual_root); + return ctx.cfg.virtual_root; else return ctx.cfg.script_name; } @@ -65,7 +65,7 @@ const char *cgit_rooturl() char *cgit_repourl(const char *reponame) { if (ctx.cfg.virtual_root) { - return fmt("%s/%s/", ctx.cfg.virtual_root, reponame); + return fmt("%s%s/", ctx.cfg.virtual_root, reponame); } else { return fmt("?r=%s", reponame); } @@ -78,7 +78,7 @@ char *cgit_fileurl(const char *reponame, const char *pagename, char *delim; if (ctx.cfg.virtual_root) { - tmp = fmt("%s/%s/%s/%s", ctx.cfg.virtual_root, reponame, + tmp = fmt("%s%s/%s/%s", ctx.cfg.virtual_root, reponame, pagename, (filename ? filename:"")); delim = "?"; } else { @@ -126,11 +126,9 @@ static void site_url(const char *page, const char *search, const char *sort, int { char *delim = "?"; - if (ctx.cfg.virtual_root) { + if (ctx.cfg.virtual_root) html_attr(ctx.cfg.virtual_root); - if (ctx.cfg.virtual_root[strlen(ctx.cfg.virtual_root) - 1] != '/') - html("/"); - } else + else html(ctx.cfg.script_name); if (page) { @@ -201,8 +199,6 @@ static char *repolink(const char *title, const char *class, const char *page, html(" href='"); if (ctx.cfg.virtual_root) { html_url_path(ctx.cfg.virtual_root); - if (ctx.cfg.virtual_root[strlen(ctx.cfg.virtual_root) - 1] != '/') - html("/"); html_url_path(ctx.repo->url); if (ctx.repo->url[strlen(ctx.repo->url) - 1] != '/') html("/"); -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 02/22] Mark char* fields in struct cgit_page as const 2013-04-07 14:26 ` [PATCH v2 00/22] " john 2013-04-07 14:26 ` [PATCH v2 01/22] Fix out-of-bounds memory accesses with virtual_root="" john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 03/22] Remove redundant calls to fmt("%s", ...) john ` (19 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- cgit.h | 12 ++++++------ ui-plain.c | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/cgit.h b/cgit.h index fc3fc6f..7581cc1 100644 --- a/cgit.h +++ b/cgit.h @@ -245,13 +245,13 @@ struct cgit_page { time_t modified; time_t expires; size_t size; - char *mimetype; - char *charset; - char *filename; - char *etag; - char *title; + const char *mimetype; + const char *charset; + const char *filename; + const char *etag; + const char *title; int status; - char *statusmsg; + const char *statusmsg; }; struct cgit_environment { diff --git a/ui-plain.c b/ui-plain.c index 4397a59..482d53a 100644 --- a/ui-plain.c +++ b/ui-plain.c @@ -100,8 +100,9 @@ static int print_object(const unsigned char *sha1, const char *path) ctx.page.etag = sha1_to_hex(sha1); cgit_print_http_headers(&ctx); html_raw(buf, size); + /* If we allocated this, then casting away const is safe. */ if (freemime) - free(ctx.page.mimetype); + free((char*) ctx.page.mimetype); return 1; } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 03/22] Remove redundant calls to fmt("%s", ...) 2013-04-07 14:26 ` [PATCH v2 00/22] " john 2013-04-07 14:26 ` [PATCH v2 01/22] Fix out-of-bounds memory accesses with virtual_root="" john 2013-04-07 14:26 ` [PATCH v2 02/22] Mark char* fields in struct cgit_page as const john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 04/22] html.c: add fmtalloc helper john ` (18 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) After this change there is one remaining call 'fmt("%s", delim)' in ui-shared.c but is needed as delim is stack allocated and so cannot be returned from the function. Signed-off-by: John Keeping <john at keeping.me.uk> --- Changes since v1: - Fix a thinko: 'fmt("%s", path)' is equivalent to path not "%s". scan-tree.c | 4 ++-- ui-plain.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scan-tree.c b/scan-tree.c index 0d3e0ad..05caba5 100644 --- a/scan-tree.c +++ b/scan-tree.c @@ -96,9 +96,9 @@ static void add_repo(const char *base, const char *path, repo_config_fn fn) return; if (base == path) - rel = xstrdup(fmt("%s", path)); + rel = xstrdup(path); else - rel = xstrdup(fmt("%s", path + strlen(base) + 1)); + rel = xstrdup(path + strlen(base) + 1); if (!strcmp(rel + strlen(rel) - 5, "/.git")) rel[strlen(rel) - 5] = '\0'; diff --git a/ui-plain.c b/ui-plain.c index 482d53a..6b0d84b 100644 --- a/ui-plain.c +++ b/ui-plain.c @@ -95,7 +95,7 @@ static int print_object(const unsigned char *sha1, const char *path) else ctx.page.mimetype = "text/plain"; } - ctx.page.filename = fmt("%s", path); + ctx.page.filename = path; ctx.page.size = size; ctx.page.etag = sha1_to_hex(sha1); cgit_print_http_headers(&ctx); -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 04/22] html.c: add fmtalloc helper 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (2 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 03/22] Remove redundant calls to fmt("%s", ...) john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 05/22] shared.c: add strbuf_ensure_end john ` (17 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- cgit.h | 3 +++ html.c | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/cgit.h b/cgit.h index 7581cc1..7619cbb 100644 --- a/cgit.h +++ b/cgit.h @@ -327,6 +327,9 @@ extern void cgit_diff_commit(struct commit *commit, filepair_fn fn, __attribute__((format (printf,1,2))) extern char *fmt(const char *format,...); +__attribute__((format (printf,1,2))) +extern char *fmtalloc(const char *format,...); + extern struct commitinfo *cgit_parse_commit(struct commit *commit); extern struct taginfo *cgit_parse_tag(struct tag *tag); extern void cgit_parse_url(const char *url); diff --git a/html.c b/html.c index 8c45ba6..c0cb4b7 100644 --- a/html.c +++ b/html.c @@ -63,6 +63,18 @@ char *fmt(const char *format, ...) return buf[bufidx]; } +char *fmtalloc(const char *format, ...) +{ + struct strbuf sb = STRBUF_INIT; + va_list args; + + va_start(args, format); + strbuf_vaddf(&sb, format, args); + va_end(args); + + return strbuf_detach(&sb, NULL); +} + void html_raw(const char *data, size_t size) { if (write(htmlfd, data, size) != size) -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 05/22] shared.c: add strbuf_ensure_end 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (3 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 04/22] html.c: add fmtalloc helper john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 06/22] cache.c: don't use statically sized buffers for filenames john ` (16 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) This is a small helper so that we can easily ensure that a strbuf ends with the specified character. Signed-off-by: John Keeping <john at keeping.me.uk> --- cgit.h | 2 ++ shared.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/cgit.h b/cgit.h index 7619cbb..3e9d55b 100644 --- a/cgit.h +++ b/cgit.h @@ -304,6 +304,8 @@ extern char *ensure_end(const char *str, char c); extern char *strlpart(char *txt, int maxlen); extern char *strrpart(char *txt, int maxlen); +extern void strbuf_ensure_end(struct strbuf *sb, char c); + extern void cgit_add_ref(struct reflist *list, struct refinfo *ref); extern void cgit_free_reflist_inner(struct reflist *list); extern int cgit_refs_cb(const char *refname, const unsigned char *sha1, diff --git a/shared.c b/shared.c index 1fa9c99..10be355 100644 --- a/shared.c +++ b/shared.c @@ -130,6 +130,12 @@ char *ensure_end(const char *str, char c) return result; } +void strbuf_ensure_end(struct strbuf *sb, char c) +{ + if (!sb->len || sb->buf[sb->len - 1] != c) + strbuf_addch(sb, c); +} + char *strlpart(char *txt, int maxlen) { char *result; -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 06/22] cache.c: don't use statically sized buffers for filenames 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (4 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 05/22] shared.c: add strbuf_ensure_end john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 07/22] html: introduce html_txtf and html_vtxtf functions john ` (15 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Instead use "struct strbuf" from Git to remove the limit on file path length. Signed-off-by: John Keeping <john at keeping.me.uk> --- Changes since v1: - Reset the buffer length before appending each filename to the base path. cache.c | 57 ++++++++++++++++++++------------------------------------- 1 file changed, 20 insertions(+), 37 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; } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 07/22] html: introduce html_txtf and html_vtxtf functions 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (5 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 06/22] cache.c: don't use statically sized buffers for filenames john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 08/22] Convert cgit_print_error to a variadic function john ` (14 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) These takes a printf style format string like htmlf but escapes the resulting string. The html_vtxtf variant takes a va_list whereas html_txtf is variadic. Signed-off-by: John Keeping <john at keeping.me.uk> --- html.c | 28 +++++++++++++++++++++++++--- html.h | 8 +++++++- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/html.c b/html.c index c0cb4b7..cac1ed3 100644 --- a/html.c +++ b/html.c @@ -88,13 +88,35 @@ void html(const char *txt) void htmlf(const char *format, ...) { - static char buf[65536]; va_list args; + struct strbuf buf = STRBUF_INIT; va_start(args, format); - vsnprintf(buf, sizeof(buf), format, args); + strbuf_vaddf(&buf, format, args); va_end(args); - html(buf); + html(buf.buf); + strbuf_release(&buf); +} + +void html_txtf(const char *format, ...) +{ + va_list args; + + va_start(args, format); + html_vtxtf(format, args); + va_end(args); +} + +void html_vtxtf(const char *format, va_list ap) +{ + va_list cp; + struct strbuf buf = STRBUF_INIT; + + va_copy(cp, ap); + strbuf_vaddf(&buf, format, cp); + va_end(cp); + html_txt(buf.buf); + strbuf_release(&buf); } void html_status(int code, const char *msg, int more_headers) diff --git a/html.h b/html.h index bb36f37..6886a46 100644 --- a/html.h +++ b/html.h @@ -1,7 +1,7 @@ #ifndef HTML_H #define HTML_H -#include <stddef.h> +#include "cgit.h" extern void html_raw(const char *txt, size_t size); extern void html(const char *txt); @@ -9,6 +9,12 @@ extern void html(const char *txt); __attribute__((format (printf,1,2))) extern void htmlf(const char *format,...); +__attribute__((format (printf,1,2))) +extern void html_txtf(const char *format,...); + +__attribute__((format (printf,1,0))) +extern void html_vtxtf(const char *format, va_list ap); + extern void html_status(int code, const char *msg, int more_headers); extern void html_txt(const char *txt); extern void html_ntxt(int len, const char *txt); -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 08/22] Convert cgit_print_error to a variadic function 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (6 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 07/22] html: introduce html_txtf and html_vtxtf functions john @ 2013-04-07 14:26 ` john 2013-04-07 15:01 ` [PATCH 08/22 v3] " john 2013-04-07 14:26 ` [PATCH v2 09/22] scan-tree: use struct strbuf instead of static buffers john ` (13 subsequent siblings) 21 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) This removes many uses of "fmt" which uses a fixed size static pool of fixed size buffers. Instead of relying on these, we now pass around argument lists for as long as possible before using a strbuf to render content of an arbitrary size. Signed-off-by: John Keeping <john at keeping.me.uk> --- cgit.c | 18 ++++++++---------- ui-blob.c | 8 ++++---- ui-commit.c | 4 ++-- ui-diff.c | 8 ++++---- ui-patch.c | 4 ++-- ui-shared.c | 15 +++++++++++++-- ui-shared.h | 5 ++++- ui-snapshot.c | 16 ++++++++++------ ui-stats.c | 5 ++--- ui-tag.c | 6 +++--- ui-tree.c | 13 +++++-------- 11 files changed, 57 insertions(+), 45 deletions(-) diff --git a/cgit.c b/cgit.c index 6f75db1..bf42023 100644 --- a/cgit.c +++ b/cgit.c @@ -459,7 +459,6 @@ static char *guess_defbranch(void) static int prepare_repo_cmd(struct cgit_context *ctx) { - char *tmp; unsigned char sha1[20]; int nongit = 0; int rc; @@ -470,14 +469,13 @@ static int prepare_repo_cmd(struct cgit_context *ctx) rc = errno; ctx->page.title = fmt("%s - %s", ctx->cfg.root_title, "config error"); - tmp = fmt("Failed to open %s: %s", - ctx->repo->name, - rc ? strerror(rc) : "Not a valid git repository"); ctx->repo = NULL; cgit_print_http_headers(ctx); cgit_print_docstart(ctx); cgit_print_pageheader(ctx); - cgit_print_error(tmp); + cgit_print_error("Failed to open %s: %s", + ctx->repo->name, + rc ? strerror(rc) : "Not a valid git repository"); cgit_print_docend(); return 1; } @@ -501,14 +499,14 @@ static int prepare_repo_cmd(struct cgit_context *ctx) } if (get_sha1(ctx->qry.head, sha1)) { - tmp = xstrdup(ctx->qry.head); + char *tmp = xstrdup(ctx->qry.head); ctx->qry.head = ctx->repo->defbranch; ctx->page.status = 404; ctx->page.statusmsg = "Not found"; cgit_print_http_headers(ctx); cgit_print_docstart(ctx); cgit_print_pageheader(ctx); - cgit_print_error(fmt("Invalid branch: %s", tmp)); + cgit_print_error("Invalid branch: %s", tmp); cgit_print_docend(); return 1; } @@ -550,7 +548,7 @@ static void process_request(void *cbdata) cgit_print_http_headers(ctx); cgit_print_docstart(ctx); cgit_print_pageheader(ctx); - cgit_print_error(fmt("No repository selected")); + cgit_print_error("No repository selected"); cgit_print_docend(); return; } @@ -862,7 +860,7 @@ int main(int argc, const char **argv) err = cache_process(ctx.cfg.cache_size, ctx.cfg.cache_root, ctx.qry.raw, ttl, process_request, &ctx); if (err) - cgit_print_error(fmt("Error processing page: %s (%d)", - strerror(err), err)); + cgit_print_error("Error processing page: %s (%d)", + strerror(err), err); return err; } diff --git a/ui-blob.c b/ui-blob.c index 0c6c215..1525115 100644 --- a/ui-blob.c +++ b/ui-blob.c @@ -94,12 +94,12 @@ void cgit_print_blob(const char *hex, char *path, const char *head) if (hex) { if (get_sha1_hex(hex, sha1)) { - cgit_print_error(fmt("Bad hex value: %s", hex)); + cgit_print_error("Bad hex value: %s", hex); return; } } else { if (get_sha1(head, sha1)) { - cgit_print_error(fmt("Bad ref: %s", head)); + cgit_print_error("Bad ref: %s", head); return; } } @@ -113,13 +113,13 @@ void cgit_print_blob(const char *hex, char *path, const char *head) } if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", hex)); + cgit_print_error("Bad object name: %s", hex); return; } buf = read_sha1_file(sha1, &type, &size); if (!buf) { - cgit_print_error(fmt("Error reading object %s", hex)); + cgit_print_error("Error reading object %s", hex); return; } diff --git a/ui-commit.c b/ui-commit.c index 105ef13..a392432 100644 --- a/ui-commit.c +++ b/ui-commit.c @@ -27,12 +27,12 @@ void cgit_print_commit(char *hex, const char *prefix) hex = ctx.qry.head; if (get_sha1(hex, sha1)) { - cgit_print_error(fmt("Bad object id: %s", hex)); + cgit_print_error("Bad object id: %s", hex); return; } commit = lookup_commit_reference(sha1); if (!commit) { - cgit_print_error(fmt("Bad commit reference: %s", hex)); + cgit_print_error("Bad commit reference: %s", hex); return; } info = cgit_parse_commit(commit); diff --git a/ui-diff.c b/ui-diff.c index 1115518..8b38209 100644 --- a/ui-diff.c +++ b/ui-diff.c @@ -369,12 +369,12 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, get_sha1(new_rev, new_rev_sha1); type = sha1_object_info(new_rev_sha1, &size); if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", new_rev)); + cgit_print_error("Bad object name: %s", new_rev); return; } commit = lookup_commit_reference(new_rev_sha1); if (!commit || parse_commit(commit)) { - cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(new_rev_sha1))); + cgit_print_error("Bad commit: %s", sha1_to_hex(new_rev_sha1)); return; } @@ -388,12 +388,12 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, if (!is_null_sha1(old_rev_sha1)) { type = sha1_object_info(old_rev_sha1, &size); if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", sha1_to_hex(old_rev_sha1))); + cgit_print_error("Bad object name: %s", sha1_to_hex(old_rev_sha1)); return; } commit2 = lookup_commit_reference(old_rev_sha1); if (!commit2 || parse_commit(commit2)) { - cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(old_rev_sha1))); + cgit_print_error("Bad commit: %s", sha1_to_hex(old_rev_sha1)); return; } } diff --git a/ui-patch.c b/ui-patch.c index 66def3c..fbb92cc 100644 --- a/ui-patch.c +++ b/ui-patch.c @@ -94,12 +94,12 @@ void cgit_print_patch(char *hex, const char *prefix) hex = ctx.qry.head; if (get_sha1(hex, sha1)) { - cgit_print_error(fmt("Bad object id: %s", hex)); + cgit_print_error("Bad object id: %s", hex); return; } commit = lookup_commit_reference(sha1); if (!commit) { - cgit_print_error(fmt("Bad commit reference: %s", hex)); + cgit_print_error("Bad commit reference: %s", hex); return; } info = cgit_parse_commit(commit); diff --git a/ui-shared.c b/ui-shared.c index c1f3c20..b93b77a 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -28,10 +28,21 @@ static char *http_date(time_t t) tm->tm_hour, tm->tm_min, tm->tm_sec); } -void cgit_print_error(const char *msg) +void cgit_print_error(const char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + cgit_vprint_error(fmt, ap); + va_end(ap); +} + +void cgit_vprint_error(const char *fmt, va_list ap) +{ + va_list cp; html("<div class='error'>"); - html_txt(msg); + va_copy(cp, ap); + html_vtxtf(fmt, cp); + va_end(cp); html("</div>\n"); } diff --git a/ui-shared.h b/ui-shared.h index 5f8b629..5987e77 100644 --- a/ui-shared.h +++ b/ui-shared.h @@ -52,7 +52,10 @@ extern void cgit_object_link(struct object *obj); extern void cgit_submodule_link(const char *class, char *path, const char *rev); -extern void cgit_print_error(const char *msg); +__attribute__((format (printf,1,2))) +extern void cgit_print_error(const char *fmt, ...); +__attribute__((format (printf,1,0))) +extern void cgit_vprint_error(const char *fmt, va_list ap); extern void cgit_print_date(time_t secs, const char *format, int local_time); extern void cgit_print_age(time_t t, time_t max_relative, const char *format); extern void cgit_print_http_headers(struct cgit_context *ctx); diff --git a/ui-snapshot.c b/ui-snapshot.c index 9be5dbe..a47884e 100644 --- a/ui-snapshot.c +++ b/ui-snapshot.c @@ -100,11 +100,11 @@ static int make_snapshot(const struct cgit_snapshot_format *format, unsigned char sha1[20]; if (get_sha1(hex, sha1)) { - cgit_print_error(fmt("Bad object id: %s", hex)); + cgit_print_error("Bad object id: %s", hex); return 1; } if (!lookup_commit_reference(sha1)) { - cgit_print_error(fmt("Not a commit reference: %s", hex)); + cgit_print_error("Not a commit reference: %s", hex); return 1; } ctx.page.mimetype = xstrdup(format->mimetype); @@ -154,13 +154,18 @@ static const char *get_ref_from_filename(const char *url, const char *filename, return NULL; } -static void show_error(char *msg) +__attribute__((format (printf, 1, 2))) +static void show_error(char *fmt, ...) { + va_list ap; + ctx.page.mimetype = "text/html"; cgit_print_http_headers(&ctx); cgit_print_docstart(&ctx); cgit_print_pageheader(&ctx); - cgit_print_error(msg); + va_start(ap, fmt); + cgit_vprint_error(fmt, ap); + va_end(ap); cgit_print_docend(); } @@ -177,8 +182,7 @@ void cgit_print_snapshot(const char *head, const char *hex, f = get_format(filename); if (!f) { - show_error(xstrdup(fmt("Unsupported snapshot format: %s", - filename))); + show_error("Unsupported snapshot format: %s", filename); return; } diff --git a/ui-stats.c b/ui-stats.c index 52e9b91..28b794f 100644 --- a/ui-stats.c +++ b/ui-stats.c @@ -374,12 +374,11 @@ void cgit_show_stats(struct cgit_context *ctx) i = cgit_find_stats_period(code, &period); if (!i) { - cgit_print_error(fmt("Unknown statistics type: %c", code[0])); + cgit_print_error("Unknown statistics type: %c", code[0]); return; } if (i > ctx->repo->max_stats) { - cgit_print_error(fmt("Statistics type disabled: %s", - period->name)); + cgit_print_error("Statistics type disabled: %s", period->name); return; } authors = collect_stats(ctx, period); diff --git a/ui-tag.c b/ui-tag.c index 5a22696..397e15b 100644 --- a/ui-tag.c +++ b/ui-tag.c @@ -50,18 +50,18 @@ void cgit_print_tag(char *revname) revname = ctx.qry.head; if (get_sha1(fmt("refs/tags/%s", revname), sha1)) { - cgit_print_error(fmt("Bad tag reference: %s", revname)); + cgit_print_error("Bad tag reference: %s", revname); return; } obj = parse_object(sha1); if (!obj) { - cgit_print_error(fmt("Bad object id: %s", sha1_to_hex(sha1))); + cgit_print_error("Bad object id: %s", sha1_to_hex(sha1)); return; } if (obj->type == OBJ_TAG) { tag = lookup_tag(sha1); if (!tag || parse_tag(tag) || !(info = cgit_parse_tag(tag))) { - cgit_print_error(fmt("Bad tag object: %s", revname)); + cgit_print_error("Bad tag object: %s", revname); return; } html("<table class='commit-info'>\n"); diff --git a/ui-tree.c b/ui-tree.c index d713553..aebe145 100644 --- a/ui-tree.c +++ b/ui-tree.c @@ -95,15 +95,13 @@ static void print_object(const unsigned char *sha1, char *path, const char *base type = sha1_object_info(sha1, &size); if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", - sha1_to_hex(sha1))); + cgit_print_error("Bad object name: %s", sha1_to_hex(sha1)); return; } buf = read_sha1_file(sha1, &type, &size); if (!buf) { - cgit_print_error(fmt("Error reading object %s", - sha1_to_hex(sha1))); + cgit_print_error("Error reading object %s", sha1_to_hex(sha1)); return; } @@ -209,8 +207,7 @@ static void ls_tree(const unsigned char *sha1, char *path, struct walk_tree_cont tree = parse_tree_indirect(sha1); if (!tree) { - cgit_print_error(fmt("Not a tree object: %s", - sha1_to_hex(sha1))); + cgit_print_error("Not a tree object: %s", sha1_to_hex(sha1)); return; } @@ -273,12 +270,12 @@ void cgit_print_tree(const char *rev, char *path) rev = ctx.qry.head; if (get_sha1(rev, sha1)) { - cgit_print_error(fmt("Invalid revision name: %s", rev)); + cgit_print_error("Invalid revision name: %s", rev); return; } commit = lookup_commit_reference(sha1); if (!commit || parse_commit(commit)) { - cgit_print_error(fmt("Invalid commit reference: %s", rev)); + cgit_print_error("Invalid commit reference: %s", rev); return; } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 08/22 v3] Convert cgit_print_error to a variadic function 2013-04-07 14:26 ` [PATCH v2 08/22] Convert cgit_print_error to a variadic function john @ 2013-04-07 15:01 ` john 0 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 15:01 UTC (permalink / raw) This removes many uses of "fmt" which uses a fixed size static pool of fixed size buffers. Instead of relying on these, we now pass around argument lists for as long as possible before using a strbuf to render content of an arbitrary size. Signed-off-by: John Keeping <john at keeping.me.uk> --- Changes since v2: - Don't access ctx->repo after setting it to NULL in prepare_repo_cmd: > @@ -470,14 +469,13 @@ static int prepare_repo_cmd(struct cgit_context *ctx) > rc = errno; > ctx->page.title = fmt("%s - %s", ctx->cfg.root_title, > "config error"); > - tmp = fmt("Failed to open %s: %s", > - ctx->repo->name, > - rc ? strerror(rc) : "Not a valid git repository"); > ctx->repo = NULL; > cgit_print_http_headers(ctx); > cgit_print_docstart(ctx); > cgit_print_pageheader(ctx); > - cgit_print_error(tmp); > + cgit_print_error("Failed to open %s: %s", > + ctx->repo->name, > + rc ? strerror(rc) : "Not a valid git repository"); cgit.c | 18 ++++++++---------- ui-blob.c | 8 ++++---- ui-commit.c | 4 ++-- ui-diff.c | 8 ++++---- ui-patch.c | 4 ++-- ui-shared.c | 15 +++++++++++++-- ui-shared.h | 5 ++++- ui-snapshot.c | 16 ++++++++++------ ui-stats.c | 5 ++--- ui-tag.c | 6 +++--- ui-tree.c | 13 +++++-------- 11 files changed, 57 insertions(+), 45 deletions(-) diff --git a/cgit.c b/cgit.c index 6f75db1..4e51283 100644 --- a/cgit.c +++ b/cgit.c @@ -459,7 +459,6 @@ static char *guess_defbranch(void) static int prepare_repo_cmd(struct cgit_context *ctx) { - char *tmp; unsigned char sha1[20]; int nongit = 0; int rc; @@ -467,17 +466,16 @@ static int prepare_repo_cmd(struct cgit_context *ctx) setenv("GIT_DIR", ctx->repo->path, 1); 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"); - tmp = fmt("Failed to open %s: %s", - ctx->repo->name, - rc ? strerror(rc) : "Not a valid git repository"); ctx->repo = NULL; cgit_print_http_headers(ctx); cgit_print_docstart(ctx); cgit_print_pageheader(ctx); - cgit_print_error(tmp); + cgit_print_error("Failed to open %s: %s", name, + rc ? strerror(rc) : "Not a valid git repository"); cgit_print_docend(); return 1; } @@ -501,14 +499,14 @@ static int prepare_repo_cmd(struct cgit_context *ctx) } if (get_sha1(ctx->qry.head, sha1)) { - tmp = xstrdup(ctx->qry.head); + char *tmp = xstrdup(ctx->qry.head); ctx->qry.head = ctx->repo->defbranch; ctx->page.status = 404; ctx->page.statusmsg = "Not found"; cgit_print_http_headers(ctx); cgit_print_docstart(ctx); cgit_print_pageheader(ctx); - cgit_print_error(fmt("Invalid branch: %s", tmp)); + cgit_print_error("Invalid branch: %s", tmp); cgit_print_docend(); return 1; } @@ -550,7 +548,7 @@ static void process_request(void *cbdata) cgit_print_http_headers(ctx); cgit_print_docstart(ctx); cgit_print_pageheader(ctx); - cgit_print_error(fmt("No repository selected")); + cgit_print_error("No repository selected"); cgit_print_docend(); return; } @@ -862,7 +860,7 @@ int main(int argc, const char **argv) err = cache_process(ctx.cfg.cache_size, ctx.cfg.cache_root, ctx.qry.raw, ttl, process_request, &ctx); if (err) - cgit_print_error(fmt("Error processing page: %s (%d)", - strerror(err), err)); + cgit_print_error("Error processing page: %s (%d)", + strerror(err), err); return err; } diff --git a/ui-blob.c b/ui-blob.c index 0c6c215..1525115 100644 --- a/ui-blob.c +++ b/ui-blob.c @@ -94,12 +94,12 @@ void cgit_print_blob(const char *hex, char *path, const char *head) if (hex) { if (get_sha1_hex(hex, sha1)) { - cgit_print_error(fmt("Bad hex value: %s", hex)); + cgit_print_error("Bad hex value: %s", hex); return; } } else { if (get_sha1(head, sha1)) { - cgit_print_error(fmt("Bad ref: %s", head)); + cgit_print_error("Bad ref: %s", head); return; } } @@ -113,13 +113,13 @@ void cgit_print_blob(const char *hex, char *path, const char *head) } if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", hex)); + cgit_print_error("Bad object name: %s", hex); return; } buf = read_sha1_file(sha1, &type, &size); if (!buf) { - cgit_print_error(fmt("Error reading object %s", hex)); + cgit_print_error("Error reading object %s", hex); return; } diff --git a/ui-commit.c b/ui-commit.c index 105ef13..a392432 100644 --- a/ui-commit.c +++ b/ui-commit.c @@ -27,12 +27,12 @@ void cgit_print_commit(char *hex, const char *prefix) hex = ctx.qry.head; if (get_sha1(hex, sha1)) { - cgit_print_error(fmt("Bad object id: %s", hex)); + cgit_print_error("Bad object id: %s", hex); return; } commit = lookup_commit_reference(sha1); if (!commit) { - cgit_print_error(fmt("Bad commit reference: %s", hex)); + cgit_print_error("Bad commit reference: %s", hex); return; } info = cgit_parse_commit(commit); diff --git a/ui-diff.c b/ui-diff.c index 1115518..8b38209 100644 --- a/ui-diff.c +++ b/ui-diff.c @@ -369,12 +369,12 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, get_sha1(new_rev, new_rev_sha1); type = sha1_object_info(new_rev_sha1, &size); if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", new_rev)); + cgit_print_error("Bad object name: %s", new_rev); return; } commit = lookup_commit_reference(new_rev_sha1); if (!commit || parse_commit(commit)) { - cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(new_rev_sha1))); + cgit_print_error("Bad commit: %s", sha1_to_hex(new_rev_sha1)); return; } @@ -388,12 +388,12 @@ void cgit_print_diff(const char *new_rev, const char *old_rev, if (!is_null_sha1(old_rev_sha1)) { type = sha1_object_info(old_rev_sha1, &size); if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", sha1_to_hex(old_rev_sha1))); + cgit_print_error("Bad object name: %s", sha1_to_hex(old_rev_sha1)); return; } commit2 = lookup_commit_reference(old_rev_sha1); if (!commit2 || parse_commit(commit2)) { - cgit_print_error(fmt("Bad commit: %s", sha1_to_hex(old_rev_sha1))); + cgit_print_error("Bad commit: %s", sha1_to_hex(old_rev_sha1)); return; } } diff --git a/ui-patch.c b/ui-patch.c index 66def3c..fbb92cc 100644 --- a/ui-patch.c +++ b/ui-patch.c @@ -94,12 +94,12 @@ void cgit_print_patch(char *hex, const char *prefix) hex = ctx.qry.head; if (get_sha1(hex, sha1)) { - cgit_print_error(fmt("Bad object id: %s", hex)); + cgit_print_error("Bad object id: %s", hex); return; } commit = lookup_commit_reference(sha1); if (!commit) { - cgit_print_error(fmt("Bad commit reference: %s", hex)); + cgit_print_error("Bad commit reference: %s", hex); return; } info = cgit_parse_commit(commit); diff --git a/ui-shared.c b/ui-shared.c index c1f3c20..b93b77a 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -28,10 +28,21 @@ static char *http_date(time_t t) tm->tm_hour, tm->tm_min, tm->tm_sec); } -void cgit_print_error(const char *msg) +void cgit_print_error(const char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + cgit_vprint_error(fmt, ap); + va_end(ap); +} + +void cgit_vprint_error(const char *fmt, va_list ap) +{ + va_list cp; html("<div class='error'>"); - html_txt(msg); + va_copy(cp, ap); + html_vtxtf(fmt, cp); + va_end(cp); html("</div>\n"); } diff --git a/ui-shared.h b/ui-shared.h index 5f8b629..5987e77 100644 --- a/ui-shared.h +++ b/ui-shared.h @@ -52,7 +52,10 @@ extern void cgit_object_link(struct object *obj); extern void cgit_submodule_link(const char *class, char *path, const char *rev); -extern void cgit_print_error(const char *msg); +__attribute__((format (printf,1,2))) +extern void cgit_print_error(const char *fmt, ...); +__attribute__((format (printf,1,0))) +extern void cgit_vprint_error(const char *fmt, va_list ap); extern void cgit_print_date(time_t secs, const char *format, int local_time); extern void cgit_print_age(time_t t, time_t max_relative, const char *format); extern void cgit_print_http_headers(struct cgit_context *ctx); diff --git a/ui-snapshot.c b/ui-snapshot.c index 9be5dbe..a47884e 100644 --- a/ui-snapshot.c +++ b/ui-snapshot.c @@ -100,11 +100,11 @@ static int make_snapshot(const struct cgit_snapshot_format *format, unsigned char sha1[20]; if (get_sha1(hex, sha1)) { - cgit_print_error(fmt("Bad object id: %s", hex)); + cgit_print_error("Bad object id: %s", hex); return 1; } if (!lookup_commit_reference(sha1)) { - cgit_print_error(fmt("Not a commit reference: %s", hex)); + cgit_print_error("Not a commit reference: %s", hex); return 1; } ctx.page.mimetype = xstrdup(format->mimetype); @@ -154,13 +154,18 @@ static const char *get_ref_from_filename(const char *url, const char *filename, return NULL; } -static void show_error(char *msg) +__attribute__((format (printf, 1, 2))) +static void show_error(char *fmt, ...) { + va_list ap; + ctx.page.mimetype = "text/html"; cgit_print_http_headers(&ctx); cgit_print_docstart(&ctx); cgit_print_pageheader(&ctx); - cgit_print_error(msg); + va_start(ap, fmt); + cgit_vprint_error(fmt, ap); + va_end(ap); cgit_print_docend(); } @@ -177,8 +182,7 @@ void cgit_print_snapshot(const char *head, const char *hex, f = get_format(filename); if (!f) { - show_error(xstrdup(fmt("Unsupported snapshot format: %s", - filename))); + show_error("Unsupported snapshot format: %s", filename); return; } diff --git a/ui-stats.c b/ui-stats.c index 52e9b91..28b794f 100644 --- a/ui-stats.c +++ b/ui-stats.c @@ -374,12 +374,11 @@ void cgit_show_stats(struct cgit_context *ctx) i = cgit_find_stats_period(code, &period); if (!i) { - cgit_print_error(fmt("Unknown statistics type: %c", code[0])); + cgit_print_error("Unknown statistics type: %c", code[0]); return; } if (i > ctx->repo->max_stats) { - cgit_print_error(fmt("Statistics type disabled: %s", - period->name)); + cgit_print_error("Statistics type disabled: %s", period->name); return; } authors = collect_stats(ctx, period); diff --git a/ui-tag.c b/ui-tag.c index 5a22696..397e15b 100644 --- a/ui-tag.c +++ b/ui-tag.c @@ -50,18 +50,18 @@ void cgit_print_tag(char *revname) revname = ctx.qry.head; if (get_sha1(fmt("refs/tags/%s", revname), sha1)) { - cgit_print_error(fmt("Bad tag reference: %s", revname)); + cgit_print_error("Bad tag reference: %s", revname); return; } obj = parse_object(sha1); if (!obj) { - cgit_print_error(fmt("Bad object id: %s", sha1_to_hex(sha1))); + cgit_print_error("Bad object id: %s", sha1_to_hex(sha1)); return; } if (obj->type == OBJ_TAG) { tag = lookup_tag(sha1); if (!tag || parse_tag(tag) || !(info = cgit_parse_tag(tag))) { - cgit_print_error(fmt("Bad tag object: %s", revname)); + cgit_print_error("Bad tag object: %s", revname); return; } html("<table class='commit-info'>\n"); diff --git a/ui-tree.c b/ui-tree.c index d713553..aebe145 100644 --- a/ui-tree.c +++ b/ui-tree.c @@ -95,15 +95,13 @@ static void print_object(const unsigned char *sha1, char *path, const char *base type = sha1_object_info(sha1, &size); if (type == OBJ_BAD) { - cgit_print_error(fmt("Bad object name: %s", - sha1_to_hex(sha1))); + cgit_print_error("Bad object name: %s", sha1_to_hex(sha1)); return; } buf = read_sha1_file(sha1, &type, &size); if (!buf) { - cgit_print_error(fmt("Error reading object %s", - sha1_to_hex(sha1))); + cgit_print_error("Error reading object %s", sha1_to_hex(sha1)); return; } @@ -209,8 +207,7 @@ static void ls_tree(const unsigned char *sha1, char *path, struct walk_tree_cont tree = parse_tree_indirect(sha1); if (!tree) { - cgit_print_error(fmt("Not a tree object: %s", - sha1_to_hex(sha1))); + cgit_print_error("Not a tree object: %s", sha1_to_hex(sha1)); return; } @@ -273,12 +270,12 @@ void cgit_print_tree(const char *rev, char *path) rev = ctx.qry.head; if (get_sha1(rev, sha1)) { - cgit_print_error(fmt("Invalid revision name: %s", rev)); + cgit_print_error("Invalid revision name: %s", rev); return; } commit = lookup_commit_reference(sha1); if (!commit || parse_commit(commit)) { - cgit_print_error(fmt("Invalid commit reference: %s", rev)); + cgit_print_error("Invalid commit reference: %s", rev); return; } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 09/22] scan-tree: use struct strbuf instead of static buffers 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (7 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 08/22] Convert cgit_print_error to a variadic function john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 10/22] ui-log.c: use a strbuf for refs john ` (12 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) 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 '/'). Signed-off-by: John Keeping <john at keeping.me.uk> --- Changes since v1: - Free the strbuf in is_git_dir. - Free the strbuf in scan_projects. scan-tree.c | 160 +++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 89 insertions(+), 71 deletions(-) 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) -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 10/22] ui-log.c: use a strbuf for refs 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (8 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 09/22] scan-tree: use struct strbuf instead of static buffers john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 11/22] ui-log.c: use a strbuf for grep arguments john ` (11 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) This avoids using a fixed-size buffer in fmt() for a user-supplied ref string. Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-log.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/ui-log.c b/ui-log.c index d75d7bf..606b67b 100644 --- a/ui-log.c +++ b/ui-log.c @@ -244,15 +244,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; } @@ -285,6 +289,7 @@ 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; + int must_free_tip = 0; char *arg; /* First argv is NULL */ @@ -292,7 +297,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern 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) { @@ -430,4 +435,8 @@ 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("</td></tr>\n"); } + + /* If we allocated tip then it is safe to cast away const. */ + if (must_free_tip) + free((char*) tip); } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 11/22] ui-log.c: use a strbuf for grep arguments 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (9 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 10/22] ui-log.c: use a strbuf for refs john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 12/22] ui-plain.c: use struct strbuf instead of fmt() john ` (10 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-log.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ui-log.c b/ui-log.c index 606b67b..4a05b0f 100644 --- a/ui-log.c +++ b/ui-log.c @@ -290,7 +290,7 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern struct vector vec = VECTOR_INIT(char *); int i, columns = commit_graph ? 4 : 3; int must_free_tip = 0; - char *arg; + struct strbuf argbuf = STRBUF_INIT; /* First argv is NULL */ vector_push(&vec, NULL, 0); @@ -304,10 +304,11 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *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 @@ -342,8 +343,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); } @@ -439,4 +440,5 @@ void cgit_print_log(const char *tip, int ofs, int cnt, char *grep, char *pattern /* If we allocated tip then it is safe to cast away const. */ if (must_free_tip) free((char*) tip); + strbuf_release(&argbuf); } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 12/22] ui-plain.c: use struct strbuf instead of fmt() 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (10 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 11/22] ui-log.c: use a strbuf for grep arguments john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 13/22] ui-refs.c: use struct strbuf instead of fixed-size buffers john ` (9 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- Changes since v1: - Use fmtalloc where appropriate ui-plain.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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("</li>\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("</li>\n"); + free(fullpath); } static void print_dir_tail(void) -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 13/22] ui-refs.c: use struct strbuf instead of fixed-size buffers 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (11 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 12/22] ui-plain.c: use struct strbuf instead of fmt() john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 14/22] ui-repolist.c: use struct strbuf for repository paths john ` (8 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- Changes since v1: - Reset filename buffer in the print_tag_downloads loop. - Use fmtalloc where appropriate ui-refs.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/ui-refs.c b/ui-refs.c index 5bebed1..ac9ab46 100644 --- a/ui-refs.c +++ b/ui-refs.c @@ -102,7 +102,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; @@ -114,7 +114,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; } } @@ -122,13 +122,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) { -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 14/22] ui-repolist.c: use struct strbuf for repository paths 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (12 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 13/22] ui-refs.c: use struct strbuf instead of fixed-size buffers john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 15/22] ui-snapshot.c: tidy up memory management in write_archive_type john ` (7 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-repolist.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) 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); } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 15/22] ui-snapshot.c: tidy up memory management in write_archive_type 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (13 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 14/22] ui-repolist.c: use struct strbuf for repository paths john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 16/22] ui-snapshot: use a struct strbuf instead of fixed-size buffers john ` (6 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) - Use a strbuf instead of a fixed-size buffer - Free the argv_array when we're done with it Note that 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 at keeping.me.uk> --- Changes since v1: - Fix freeing the argv array to avoid double frees. ui-snapshot.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/ui-snapshot.c b/ui-snapshot.c index a47884e..1b361c1 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) -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 16/22] ui-snapshot: use a struct strbuf instead of fixed-size buffers 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (14 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 15/22] ui-snapshot.c: tidy up memory management in write_archive_type john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 17/22] ui-summary.c: use " john ` (5 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- Changes since v1: - Fix completely bogus reset-then-use code. ui-snapshot.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/ui-snapshot.c b/ui-snapshot.c index 1b361c1..8e76977 100644 --- a/ui-snapshot.c +++ b/ui-snapshot.c @@ -148,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))) -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 17/22] ui-summary.c: use struct strbuf instead of fixed-size buffers 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (15 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 16/22] ui-snapshot: use a struct strbuf instead of fixed-size buffers john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 18/22] ui-tag.c: use struct strbuf for user-supplied data john ` (4 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- Changes since v1: - Use fmtalloc where appropriate ui-summary.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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("<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 @@ 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. -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 18/22] ui-tag.c: use struct strbuf for user-supplied data 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (16 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 17/22] ui-summary.c: use " john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 19/22] ui-tree.c: use struct strbuf instead of fmt() john ` (3 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-tag.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) 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("<table class='commit-info'>\n"); htmlf("<tr><td>tag name</td><td>"); @@ -101,5 +103,7 @@ void cgit_print_tag(char *revname) print_download_links(revname); html("</table>\n"); } - return; + +cleanup: + strbuf_release(&fullref); } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 19/22] ui-tree.c: use struct strbuf instead of fmt() 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (17 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 18/22] ui-tag.c: use struct strbuf for user-supplied data john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 20/22] cgit.c: " john ` (2 subsequent siblings) 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- ui-tree.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) 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("</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; } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 20/22] cgit.c: use struct strbuf instead of fmt() 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (18 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 19/22] ui-tree.c: use struct strbuf instead of fmt() john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 21/22] html: add html_attrf to output an attribute value from a format string john 2013-04-07 14:26 ` [PATCH v2 22/22] ui-shared.c: use struct strbuf instead of fmt() john 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- Changes since v1: - Use fmtalloc where appropriate cgit.c | 72 +++++++++++++++++++++++++++++++++--------------------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/cgit.c b/cgit.c index bf42023..532fc17 100644 --- a/cgit.c +++ b/cgit.c @@ -467,8 +467,8 @@ static int prepare_repo_cmd(struct cgit_context *ctx) setup_git_directory_gently(&nongit); if (nongit) { 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); -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 21/22] html: add html_attrf to output an attribute value from a format string 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (19 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 20/22] cgit.c: " john @ 2013-04-07 14:26 ` john 2013-04-07 14:26 ` [PATCH v2 22/22] ui-shared.c: use struct strbuf instead of fmt() john 21 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- html.c | 13 +++++++++++++ html.h | 3 +++ 2 files changed, 16 insertions(+) diff --git a/html.c b/html.c index cac1ed3..f7772dc 100644 --- a/html.c +++ b/html.c @@ -170,6 +170,19 @@ void html_ntxt(int len, const char *txt) html("..."); } +void html_attrf(const char *fmt, ...) +{ + va_list ap; + struct strbuf sb = STRBUF_INIT; + + va_start(ap, fmt); + strbuf_vaddf(&sb, fmt, ap); + va_end(ap); + + html_attr(sb.buf); + strbuf_release(&sb); +} + void html_attr(const char *txt) { const char *t = txt; diff --git a/html.h b/html.h index 6886a46..be3b311 100644 --- a/html.h +++ b/html.h @@ -15,6 +15,9 @@ extern void html_txtf(const char *format,...); __attribute__((format (printf,1,0))) extern void html_vtxtf(const char *format, va_list ap); +__attribute__((format (printf,1,2))) +extern void html_attrf(const char *format,...); + extern void html_status(int code, const char *msg, int more_headers); extern void html_txt(const char *txt); extern void html_ntxt(int len, const char *txt); -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 22/22] ui-shared.c: use struct strbuf instead of fmt() 2013-04-07 14:26 ` [PATCH v2 00/22] " john ` (20 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 21/22] html: add html_attrf to output an attribute value from a format string john @ 2013-04-07 14:26 ` john 2013-04-07 15:21 ` Jason 21 siblings, 1 reply; 75+ messages in thread From: john @ 2013-04-07 14:26 UTC (permalink / raw) Signed-off-by: John Keeping <john at keeping.me.uk> --- Changes since v1: - Catch another fmt() in cgit_add_hidden_formfields - Catch another fmt() in cgit_print_snapshot_links - Use fmtalloc where appropriate ui-shared.c | 64 ++++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/ui-shared.c b/ui-shared.c index b93b77a..468f47a 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -56,13 +56,14 @@ const char *cgit_httpscheme() const char *cgit_hosturl() { + struct strbuf sb = STRBUF_INIT; if (ctx.env.http_host) return ctx.env.http_host; 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 +76,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 +548,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("</a>"); - html_txt(fmt(" @ %.7s", rev)); + html_txtf(" @ %.7s", rev); if (item && tail) path[len - 1] = tail; } @@ -678,12 +678,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("<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 +729,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 +931,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); } -- 1.8.2.692.g17a9715 ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 22/22] ui-shared.c: use struct strbuf instead of fmt() 2013-04-07 14:26 ` [PATCH v2 22/22] ui-shared.c: use struct strbuf instead of fmt() john @ 2013-04-07 15:21 ` Jason 2013-04-07 15:43 ` john 0 siblings, 1 reply; 75+ messages in thread From: Jason @ 2013-04-07 15:21 UTC (permalink / raw) On Sun, Apr 7, 2013 at 4:26 PM, John Keeping <john at keeping.me.uk> wrote: > const char *cgit_hosturl() > { > + struct strbuf sb = STRBUF_INIT; This is now unused. > if (ctx.env.http_host) > return ctx.env.http_host; > 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); > } ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 22/22] ui-shared.c: use struct strbuf instead of fmt() 2013-04-07 15:21 ` Jason @ 2013-04-07 15:43 ` john 2013-04-07 15:46 ` Jason 2013-04-08 10:22 ` cgit 0 siblings, 2 replies; 75+ messages in thread From: john @ 2013-04-07 15:43 UTC (permalink / raw) On Sun, Apr 07, 2013 at 05:21:28PM +0200, Jason A. Donenfeld wrote: > On Sun, Apr 7, 2013 at 4:26 PM, John Keeping <john at keeping.me.uk> wrote: > > const char *cgit_hosturl() > > { > > + struct strbuf sb = STRBUF_INIT; > > This is now unused. Thanks. Fixed and pushed to my GitHub repository [1]. I'll avoid sending another reroll of the entire series and only send incremental patches here if nothing serious is found. [1] http://github.com/johnkeeping/cgit > > if (ctx.env.http_host) > > return ctx.env.http_host; > > 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); > > } ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 22/22] ui-shared.c: use struct strbuf instead of fmt() 2013-04-07 15:43 ` john @ 2013-04-07 15:46 ` Jason 2013-04-08 10:22 ` cgit 1 sibling, 0 replies; 75+ messages in thread From: Jason @ 2013-04-07 15:46 UTC (permalink / raw) On Sun, Apr 7, 2013 at 5:43 PM, John Keeping <john at keeping.me.uk> wrote: > Thanks. Fixed and pushed to my GitHub repository [1]. I'll avoid > sending another reroll of the entire series and only send incremental > patches here if nothing serious is found. > > [1] http://github.com/johnkeeping/cgit Sounds good, thanks. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 22/22] ui-shared.c: use struct strbuf instead of fmt() 2013-04-07 15:43 ` john 2013-04-07 15:46 ` Jason @ 2013-04-08 10:22 ` cgit 2013-04-08 14:04 ` Jason 1 sibling, 1 reply; 75+ messages in thread From: cgit @ 2013-04-08 10:22 UTC (permalink / raw) On Sun, Apr 07, 2013 at 04:43:26PM +0100, John Keeping wrote: > On Sun, Apr 07, 2013 at 05:21:28PM +0200, Jason A. Donenfeld wrote: > > On Sun, Apr 7, 2013 at 4:26 PM, John Keeping <john at keeping.me.uk> wrote: > > > const char *cgit_hosturl() > > > { > > > + struct strbuf sb = STRBUF_INIT; > > > > This is now unused. > > Thanks. Fixed and pushed to my GitHub repository [1]. I'll avoid > sending another reroll of the entire series and only send incremental > patches here if nothing serious is found. Note that you added the patch history to some of the commit messages for some reason... > > [1] http://github.com/johnkeeping/cgit > > > > if (ctx.env.http_host) > > > return ctx.env.http_host; > > > 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); > > > } > > _______________________________________________ > cgit mailing list > cgit at hjemli.net > http://hjemli.net/mailman/listinfo/cgit ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 22/22] ui-shared.c: use struct strbuf instead of fmt() 2013-04-08 10:22 ` cgit @ 2013-04-08 14:04 ` Jason 2013-04-08 17:40 ` john 0 siblings, 1 reply; 75+ messages in thread From: Jason @ 2013-04-08 14:04 UTC (permalink / raw) On Mon, Apr 8, 2013 at 12:22 PM, Lukas Fleischer <cgit at cryptocrack.de> wrote: > On Sun, Apr 07, 2013 at 04:43:26PM +0100, John Keeping wrote: >> Thanks. Fixed and pushed to my GitHub repository [1]. I'll avoid >> sending another reroll of the entire series and only send incremental >> patches here if nothing serious is found. > > Note that you added the patch history to some of the commit messages for > some reason... I've got this fixed in the branch here -- http://git.zx2c4.com/cgit/log/?h=jk/fixed-size-buffer As well, this branch is now rebased against master, which recently received a considerable merge from wip. ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH v2 22/22] ui-shared.c: use struct strbuf instead of fmt() 2013-04-08 14:04 ` Jason @ 2013-04-08 17:40 ` john 0 siblings, 0 replies; 75+ messages in thread From: john @ 2013-04-08 17:40 UTC (permalink / raw) On Mon, Apr 08, 2013 at 04:04:49PM +0200, Jason A. Donenfeld wrote: > On Mon, Apr 8, 2013 at 12:22 PM, Lukas Fleischer <cgit at cryptocrack.de> wrote: > > On Sun, Apr 07, 2013 at 04:43:26PM +0100, John Keeping wrote: > >> Thanks. Fixed and pushed to my GitHub repository [1]. I'll avoid > >> sending another reroll of the entire series and only send incremental > >> patches here if nothing serious is found. > > > > Note that you added the patch history to some of the commit messages for > > some reason... > > I've got this fixed in the branch here -- > http://git.zx2c4.com/cgit/log/?h=jk/fixed-size-buffer > > As well, this branch is now rebased against master, which recently > received a considerable merge from wip. Thanks. I tend to use "git commit --squash=..." and list the changes in the commit message, to be tidied while running git-format-patch. I know notes are the correct way to do that, I just need to retrain myself to use them. :-) ^ permalink raw reply [flat|nested] 75+ messages in thread
* [PATCH 00/19] Fixed-size buffer removal 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john ` (20 preceding siblings ...) 2013-04-07 14:26 ` [PATCH v2 00/22] " john @ 2013-04-08 14:23 ` Jason 21 siblings, 0 replies; 75+ messages in thread From: Jason @ 2013-04-08 14:23 UTC (permalink / raw) On Sun, Apr 7, 2013 at 11:29 AM, John Keeping <john at keeping.me.uk> wrote: > This series replaces use of fixed-size buffers for any user-supplied > input or data from the repository with Git's struct strbuf. It is based > on wip. I've rebased the latest of this series into wip: http://git.zx2c4.com/cgit/log/?h=staging/jk/fixed-size-buffer http://git.zx2c4.com/cgit/log/?h=wip Lest any new issues crop up in the next span of time, I'll be merging this into master. ^ permalink raw reply [flat|nested] 75+ messages in thread
end of thread, other threads:[~2013-04-08 18:28 UTC | newest] Thread overview: 75+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-04-07 9:29 [PATCH 00/19] Fixed-size buffer removal john 2013-04-07 9:29 ` [PATCH 01/19] Fix out-of-bounds memory accesses with virtual_root="" john 2013-04-07 9:29 ` [PATCH 02/19] Remove redundant calls to fmt("%s", ...) john 2013-04-07 11:05 ` Jason 2013-04-07 11:25 ` john 2013-04-07 11:30 ` Jason 2013-04-07 9:29 ` [PATCH 03/19] cache.c: don't use statically sized buffers for filenames john 2013-04-07 11:11 ` Jason 2013-04-07 11:30 ` john 2013-04-07 9:29 ` [PATCH 04/19] html: introduce html_txtf and html_vtxtf functions john 2013-04-07 9:29 ` [PATCH 05/19] Convert cgit_print_error to a variadic function john 2013-04-07 9:29 ` [PATCH 06/19] scan-tree: use struct strbuf instead of static buffers john 2013-04-07 11:29 ` Jason 2013-04-07 11:33 ` john 2013-04-07 9:29 ` [PATCH 07/19] ui-log.c: use a strbuf for refs john 2013-04-07 9:29 ` [PATCH 08/19] ui-log.c: use a strbuf for grep arguments john 2013-04-07 9:30 ` [PATCH 09/19] ui-plain.c: use struct strbuf instead of fmt() john 2013-04-07 9:30 ` [PATCH 10/19] ui-refs.c: use struct strbuf instead of fixed-size buffers john 2013-04-07 13:08 ` Jason 2013-04-07 9:30 ` [PATCH 11/19] ui-repolist.c: use struct strbuf for repository paths john 2013-04-07 9:30 ` [PATCH 12/19] ui-snapshot.c: tidy up memory management in write_archive_type john 2013-04-07 9:30 ` [PATCH 13/19] ui-snapshot: use a struct strbuf instead of fixed-size buffers john 2013-04-07 13:25 ` Jason 2013-04-07 13:37 ` john 2013-04-07 13:39 ` Jason 2013-04-07 13:33 ` Jason 2013-04-07 9:30 ` [PATCH 14/19] ui-summary.c: use " john 2013-04-07 12:20 ` Jason 2013-04-07 12:36 ` john 2013-04-07 12:41 ` Jason 2013-04-07 12:43 ` Jason 2013-04-07 9:30 ` [PATCH 15/19] ui-tag.c: use struct strbuf for user-supplied data john 2013-04-07 9:30 ` [PATCH 16/19] ui-tree.c: use struct strbuf instead of fmt() john 2013-04-07 9:30 ` [PATCH 17/19] cgit.c: " john 2013-04-07 9:30 ` [PATCH 18/19] html: add html_attrf to output an attribute value from a format string john 2013-04-07 9:30 ` [PATCH 19/19] ui-shared.c: use struct strbuf instead of fmt() john 2013-04-07 12:37 ` Jason 2013-04-07 12:44 ` john 2013-04-07 12:49 ` cgit 2013-04-07 13:08 ` [PATCH 00/19] Fixed-size buffer removal Jason 2013-04-07 13:14 ` john 2013-04-08 15:31 ` Jason 2013-04-08 17:38 ` john 2013-04-08 18:28 ` Jason 2013-04-07 14:26 ` [PATCH v2 00/22] " john 2013-04-07 14:26 ` [PATCH v2 01/22] Fix out-of-bounds memory accesses with virtual_root="" john 2013-04-07 14:26 ` [PATCH v2 02/22] Mark char* fields in struct cgit_page as const john 2013-04-07 14:26 ` [PATCH v2 03/22] Remove redundant calls to fmt("%s", ...) john 2013-04-07 14:26 ` [PATCH v2 04/22] html.c: add fmtalloc helper john 2013-04-07 14:26 ` [PATCH v2 05/22] shared.c: add strbuf_ensure_end john 2013-04-07 14:26 ` [PATCH v2 06/22] cache.c: don't use statically sized buffers for filenames john 2013-04-07 14:26 ` [PATCH v2 07/22] html: introduce html_txtf and html_vtxtf functions john 2013-04-07 14:26 ` [PATCH v2 08/22] Convert cgit_print_error to a variadic function john 2013-04-07 15:01 ` [PATCH 08/22 v3] " john 2013-04-07 14:26 ` [PATCH v2 09/22] scan-tree: use struct strbuf instead of static buffers john 2013-04-07 14:26 ` [PATCH v2 10/22] ui-log.c: use a strbuf for refs john 2013-04-07 14:26 ` [PATCH v2 11/22] ui-log.c: use a strbuf for grep arguments john 2013-04-07 14:26 ` [PATCH v2 12/22] ui-plain.c: use struct strbuf instead of fmt() john 2013-04-07 14:26 ` [PATCH v2 13/22] ui-refs.c: use struct strbuf instead of fixed-size buffers john 2013-04-07 14:26 ` [PATCH v2 14/22] ui-repolist.c: use struct strbuf for repository paths john 2013-04-07 14:26 ` [PATCH v2 15/22] ui-snapshot.c: tidy up memory management in write_archive_type john 2013-04-07 14:26 ` [PATCH v2 16/22] ui-snapshot: use a struct strbuf instead of fixed-size buffers john 2013-04-07 14:26 ` [PATCH v2 17/22] ui-summary.c: use " john 2013-04-07 14:26 ` [PATCH v2 18/22] ui-tag.c: use struct strbuf for user-supplied data john 2013-04-07 14:26 ` [PATCH v2 19/22] ui-tree.c: use struct strbuf instead of fmt() john 2013-04-07 14:26 ` [PATCH v2 20/22] cgit.c: " john 2013-04-07 14:26 ` [PATCH v2 21/22] html: add html_attrf to output an attribute value from a format string john 2013-04-07 14:26 ` [PATCH v2 22/22] ui-shared.c: use struct strbuf instead of fmt() john 2013-04-07 15:21 ` Jason 2013-04-07 15:43 ` john 2013-04-07 15:46 ` Jason 2013-04-08 10:22 ` cgit 2013-04-08 14:04 ` Jason 2013-04-08 17:40 ` john 2013-04-08 14:23 ` [PATCH 00/19] Fixed-size buffer removal Jason
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).