List for cgit developers and users
 help / color / mirror / Atom feed
From: john at keeping.me.uk (John Keeping)
Subject: [PATCH 08/22 v3] Convert cgit_print_error to a variadic function
Date: Sun, 7 Apr 2013 16:01:12 +0100	[thread overview]
Message-ID: <20130407150112.GD2222@serenity.lan> (raw)
In-Reply-To: <f2dda278b7f89a8bedea48aa3696d4180e33bf43.1365344206.git.john@keeping.me.uk>

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





  reply	other threads:[~2013-04-07 15:01 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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     ` john [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130407150112.GD2222@serenity.lan \
    --to=cgit@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).