[master] c4c8f2f Split the datastructures we use to construct bans (ban_proto) from the data structure we use to store and operate on the ban (ban).

Poul-Henning Kamp phk at FreeBSD.org
Thu Oct 22 17:40:38 CEST 2015


commit c4c8f2fae16fe888b9745442e708ad6dfcc91d93
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Thu Oct 22 15:39:51 2015 +0000

    Split the datastructures we use to construct bans (ban_proto)
    from the data structure we use to store and operate on the ban (ban).

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 11c8be1..93e5de4 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -102,6 +102,7 @@ enum {
 
 struct VSC_C_lck;
 struct ban;
+struct ban_proto;
 struct backend;
 struct busyobj;
 struct cli;
@@ -657,11 +658,11 @@ typedef enum htc_status_e htc_complete_f(struct http_conn *);
 /* cache_ban.c */
 
 /* for constructing bans */
-struct ban *BAN_New(void);
-int BAN_AddTest(struct ban *, const char *, const char *, const char *);
-void BAN_Free(struct ban *b);
-char *BAN_Insert(struct ban *b);
-void BAN_Free_Errormsg(char *);
+struct ban_proto *BAN_Build(void);
+const char *BAN_AddTest(struct ban_proto *,
+    const char *, const char *, const char *);
+const char *BAN_Commit(struct ban_proto *b);
+void BAN_Abandon(struct ban_proto *b);
 
 /* for stevedoes resurrecting bans */
 void BAN_Hold(void);
diff --git a/bin/varnishd/cache/cache_ban.c b/bin/varnishd/cache/cache_ban.c
index 0247d44..95ca144 100644
--- a/bin/varnishd/cache/cache_ban.c
+++ b/bin/varnishd/cache/cache_ban.c
@@ -45,7 +45,6 @@ struct lock ban_mtx;
 int ban_shutdown;
 struct banhead_s ban_head = VTAILQ_HEAD_INITIALIZER(ban_head);
 struct ban * volatile ban_start;
-struct ban *ban_magic;
 
 static pthread_t ban_thread;
 static int ban_holds;
@@ -73,23 +72,6 @@ ban_alloc(void)
 	return (b);
 }
 
-struct ban *
-BAN_New(void)
-{
-	struct ban *b;
-
-	b = ban_alloc();
-	if (b != NULL) {
-		b->vsb = VSB_new_auto();
-		if (b->vsb == NULL) {
-			FREE_OBJ(b);
-			return (NULL);
-		}
-		VTAILQ_INIT(&b->objcore);
-	}
-	return (b);
-}
-
 void
 BAN_Free(struct ban *b)
 {
@@ -98,8 +80,6 @@ BAN_Free(struct ban *b)
 	AZ(b->refcount);
 	assert(VTAILQ_EMPTY(&b->objcore));
 
-	if (b->vsb != NULL)
-		VSB_delete(b->vsb);
 	if (b->spec != NULL)
 		free(b->spec);
 	FREE_OBJ(b);
@@ -595,8 +575,8 @@ static void
 ccf_ban(struct cli *cli, const char * const *av, void *priv)
 {
 	int narg, i;
-	struct ban *b;
-	char *p;
+	struct ban_proto *bp;
+	const char *err = NULL;
 
 	(void)priv;
 
@@ -616,19 +596,24 @@ ccf_ban(struct cli *cli, const char * const *av, void *priv)
 		}
 	}
 
-	b = BAN_New();
-	if (b == NULL) {
+	bp = BAN_Build();
+	if (bp == NULL) {
 		VCLI_Out(cli, "Out of Memory");
 		VCLI_SetResult(cli, CLIS_CANT);
 		return;
 	}
-	for (i = 0; i < narg; i += 4)
-		if (BAN_AddTest(b, av[i + 2], av[i + 3], av[i + 4]))
+	for (i = 0; i < narg; i += 4) {
+		err = BAN_AddTest(bp, av[i + 2], av[i + 3], av[i + 4]);
+		if (err)
 			break;
-	p = BAN_Insert(b);
-	if (p != NULL) {
-		VCLI_Out(cli, "%s", p);
-		BAN_Free_Errormsg(p);
+	}
+
+	if (err == NULL)
+		err = BAN_Commit(bp);
+
+	if (err != NULL) {
+		VCLI_Out(cli, "%s", err);
+		BAN_Abandon(bp);
 		VCLI_SetResult(cli, CLIS_PARAM);
 	}
 }
@@ -679,6 +664,7 @@ static void
 ccf_ban_list(struct cli *cli, const char * const *av, void *priv)
 {
 	struct ban *b, *bl;
+	int64_t o;
 
 	(void)av;
 	(void)priv;
@@ -691,14 +677,14 @@ ccf_ban_list(struct cli *cli, const char * const *av, void *priv)
 
 	VCLI_Out(cli, "Present bans:\n");
 	VTAILQ_FOREACH(b, &ban_head, list) {
-		VCLI_Out(cli, "%10.6f %5u %s", ban_time(b->spec),
-		    bl == b ? b->refcount - 1 : b->refcount,
+		o = bl == b ? 1 : 0;
+		VCLI_Out(cli, "%10.6f %5ju %s", ban_time(b->spec),
+		    (intmax_t)b->refcount - o,
 		    b->flags & BANS_FLAG_COMPLETED ? "C" : " ");
 		if (DO_DEBUG(DBG_LURKER)) {
-			VCLI_Out(cli, "%s%s%s %p ",
+			VCLI_Out(cli, "%s%s %p ",
 			    b->flags & BANS_FLAG_REQ ? "R" : "-",
 			    b->flags & BANS_FLAG_OBJ ? "O" : "-",
-			    b->flags & BANS_FLAG_ERROR ? "E" : "-",
 			    b);
 		}
 		VCLI_Out(cli, "  ");
@@ -732,6 +718,7 @@ static struct cli_proto ban_cmds[] = {
 void
 BAN_Compile(void)
 {
+	struct ban *b;
 
 	/*
 	 * All bans have been read from all persistent stevedores. Export
@@ -743,8 +730,9 @@ BAN_Compile(void)
 
 	Lck_Lock(&ban_mtx);
 
-	/* Do late reporting of ban_magic */
-	AZ(STV_BanInfo(BI_NEW, ban_magic->spec, ban_len(ban_magic->spec)));
+	/* Report the place-holder ban */
+	b = VTAILQ_FIRST(&ban_head);
+	AZ(STV_BanInfo(BI_NEW, b->spec, ban_len(b->spec)));
 
 	ban_export();
 
@@ -757,16 +745,19 @@ BAN_Compile(void)
 void
 BAN_Init(void)
 {
+	struct ban_proto *bp;
 
 	Lck_New(&ban_mtx, lck_ban);
 	CLI_AddFuncs(ban_cmds);
 
-	ban_magic = BAN_New();
-	AN(ban_magic);
-	AZ(BAN_Insert(ban_magic));
-	Lck_Lock(&ban_mtx);
-	ban_mark_completed(ban_magic);
 	ban_holds = 1;
+
+	/* Add a placeholder ban */
+	bp = BAN_Build();
+	AN(bp);
+	AZ(BAN_Commit(bp));
+	Lck_Lock(&ban_mtx);
+	ban_mark_completed(VTAILQ_FIRST(&ban_head));
 	Lck_Unlock(&ban_mtx);
 }
 
diff --git a/bin/varnishd/cache/cache_ban.h b/bin/varnishd/cache/cache_ban.h
index 755b2f4..047d0f9 100644
--- a/bin/varnishd/cache/cache_ban.h
+++ b/bin/varnishd/cache/cache_ban.h
@@ -73,7 +73,6 @@
 #define BANS_FLAG_OBJ		(1<<1)
 #define BANS_FLAG_COMPLETED	(1<<2)
 #define BANS_FLAG_HTTP		(1<<3)
-#define BANS_FLAG_ERROR		(1<<4)
 
 #define BANS_OPER_EQ		0x10
 #define BANS_OPER_NEQ		0x11
@@ -90,13 +89,12 @@
 struct ban {
 	unsigned		magic;
 #define BAN_MAGIC		0x700b08ea
+	unsigned		flags;		/* BANS_FLAG_* */
 	VTAILQ_ENTRY(ban)	list;
 	VTAILQ_ENTRY(ban)	l_list;
-	int			refcount;
-	unsigned		flags;		/* BANS_FLAG_* */
+	int64_t			refcount;
 
 	VTAILQ_HEAD(,objcore)	objcore;
-	struct vsb		*vsb;
 	uint8_t			*spec;
 };
 
@@ -107,7 +105,6 @@ extern struct lock ban_mtx;
 extern int ban_shutdown;
 extern struct banhead_s ban_head;
 extern struct ban * volatile ban_start;
-extern struct ban *ban_magic;
 
 void ban_mark_completed(struct ban *b);
 unsigned ban_len(const uint8_t *banspec);
@@ -116,3 +113,4 @@ int ban_evaluate(struct worker *wrk, const uint8_t *bs, struct objcore *oc,
     const struct http *reqhttp, unsigned *tests);
 double ban_time(const uint8_t *banspec);
 int ban_equal(const uint8_t *bs1, const uint8_t *bs2);
+void BAN_Free(struct ban *b);
diff --git a/bin/varnishd/cache/cache_ban_build.c b/bin/varnishd/cache/cache_ban_build.c
index ee2f0e7..eb10c30 100644
--- a/bin/varnishd/cache/cache_ban_build.c
+++ b/bin/varnishd/cache/cache_ban_build.c
@@ -38,6 +38,15 @@
 #include "vend.h"
 #include "vtim.h"
 
+struct ban_proto {
+	unsigned		magic;
+#define BAN_PROTO_MAGIC		0xd8adc494
+	unsigned		flags;		/* BANS_FLAG_* */
+
+	struct vsb		*vsb;
+	char			*err;
+};
+
 /*--------------------------------------------------------------------
  * Variables we can purge on
  */
@@ -53,41 +62,83 @@ static const struct pvar {
 	{ 0, 0, 0}
 };
 
+/*--------------------------------------------------------------------
+ */
+
+static char ban_build_err_no_mem[] = "No Memory";
+
+/*--------------------------------------------------------------------
+ */
+
+struct ban_proto *
+BAN_Build(void)
+{
+	struct ban_proto *bp;
+
+	ALLOC_OBJ(bp, BAN_PROTO_MAGIC);
+	if (bp == NULL)
+		return (bp);
+	bp->vsb = VSB_new_auto();
+	if (bp->vsb == NULL) {
+		FREE_OBJ(bp);
+		return (NULL);
+	}
+	return (bp);
+}
+
+void
+BAN_Abandon(struct ban_proto *bp)
+{
+
+	CHECK_OBJ_NOTNULL(bp, BAN_PROTO_MAGIC);
+	VSB_delete(bp->vsb);
+	if (bp->err != NULL && bp->err != ban_build_err_no_mem)
+		REPLACE(bp->err, NULL);
+	FREE_OBJ(bp);
+}
+
+/*--------------------------------------------------------------------
+ */
+
 static void
-ban_add_lump(const struct ban *b, const void *p, uint32_t len)
+ban_add_lump(const struct ban_proto *bp, const void *p, uint32_t len)
 {
 	uint8_t buf[sizeof len];
 
 	buf[0] = 0xff;
-	while (VSB_len(b->vsb) & PALGN)
-		VSB_bcat(b->vsb, buf, 1);
+	while (VSB_len(bp->vsb) & PALGN)
+		VSB_bcat(bp->vsb, buf, 1);
 	vbe32enc(buf, len);
-	VSB_bcat(b->vsb, buf, sizeof buf);
-	VSB_bcat(b->vsb, p, len);
+	VSB_bcat(bp->vsb, buf, sizeof buf);
+	VSB_bcat(bp->vsb, p, len);
 }
 
 /*--------------------------------------------------------------------
  */
 
-static int
-ban_error(struct ban *b, const char *fmt, ...)
+static const char *
+ban_error(struct ban_proto *bp, const char *fmt, ...)
 {
 	va_list ap;
 
-	CHECK_OBJ_NOTNULL(b, BAN_MAGIC);
-	AN(b->vsb);
+	CHECK_OBJ_NOTNULL(bp, BAN_PROTO_MAGIC);
+	AN(bp->vsb);
 
 	/* First error is sticky */
-	if (!(b->flags & BANS_FLAG_ERROR)) {
-		b->flags |= BANS_FLAG_ERROR;
-
-		/* Record the error message in the vsb */
-		VSB_clear(b->vsb);
-		va_start(ap, fmt);
-		(void)VSB_vprintf(b->vsb, fmt, ap);
-		va_end(ap);
+	if (bp->err == NULL) {
+		if (fmt == ban_build_err_no_mem) {
+			bp->err = ban_build_err_no_mem;
+		} else {
+			/* Record the error message in the vsb */
+			VSB_clear(bp->vsb);
+			va_start(ap, fmt);
+			(void)VSB_vprintf(bp->vsb, fmt, ap);
+			va_end(ap);
+			AZ(VSB_finish(bp->vsb));
+			bp->err = VSB_data(bp->vsb);
+		}
 	}
-	return (-1);
+	return (bp->err);
 }
 
 /*--------------------------------------------------------------------
@@ -96,24 +147,24 @@ ban_error(struct ban *b, const char *fmt, ...)
  */
 
 static void
-ban_parse_http(const struct ban *b, const char *a1)
+ban_parse_http(const struct ban_proto *bp, const char *a1)
 {
 	int l;
 
 	l = strlen(a1) + 1;
 	assert(l <= 127);
-	VSB_putc(b->vsb, (char)l);
-	VSB_cat(b->vsb, a1);
-	VSB_putc(b->vsb, ':');
-	VSB_putc(b->vsb, '\0');
+	VSB_putc(bp->vsb, (char)l);
+	VSB_cat(bp->vsb, a1);
+	VSB_putc(bp->vsb, ':');
+	VSB_putc(bp->vsb, '\0');
 }
 
 /*--------------------------------------------------------------------
  * Parse and add a ban test specification
  */
 
-static int
-ban_parse_regexp(struct ban *b, const char *a3)
+static const char *
+ban_parse_regexp(struct ban_proto *bp, const char *a3)
 {
 	const char *error;
 	int erroroffset, rc;
@@ -122,10 +173,10 @@ ban_parse_regexp(struct ban *b, const char *a3)
 
 	re = pcre_compile(a3, 0, &error, &erroroffset, NULL);
 	if (re == NULL)
-		return (ban_error(b, "Regex compile error: %s", error));
+		return (ban_error(bp, "Regex compile error: %s", error));
 	rc = pcre_fullinfo(re, NULL, PCRE_INFO_SIZE, &sz);
 	AZ(rc);
-	ban_add_lump(b, re, sz);
+	ban_add_lump(bp, re, sz);
 	pcre_free(re);
 	return (0);
 }
@@ -134,55 +185,56 @@ ban_parse_regexp(struct ban *b, const char *a3)
  * Add a (and'ed) test-condition to a ban
  */
 
-int
-BAN_AddTest(struct ban *b, const char *a1, const char *a2, const char *a3)
+const char *
+BAN_AddTest(struct ban_proto *bp,
+    const char *a1, const char *a2, const char *a3)
 {
 	const struct pvar *pv;
-	int i;
+	const char *err;
 
-	CHECK_OBJ_NOTNULL(b, BAN_MAGIC);
-	AN(b->vsb);
+	CHECK_OBJ_NOTNULL(bp, BAN_PROTO_MAGIC);
+	AN(bp->vsb);
 	AN(a1);
 	AN(a2);
 	AN(a3);
 
-	if (b->flags & BANS_FLAG_ERROR)
-		return (-1);
+	if (bp->err != NULL)
+		return (bp->err);
 
 	for (pv = pvars; pv->name != NULL; pv++)
 		if (!strncmp(a1, pv->name, strlen(pv->name)))
 			break;
 
 	if (pv->name == NULL)
-		return (ban_error(b,
+		return (ban_error(bp,
 		    "Unknown or unsupported field \"%s\"", a1));
 
-	b->flags |= pv->flag;
+	bp->flags |= pv->flag;
 
-	VSB_putc(b->vsb, pv->tag);
+	VSB_putc(bp->vsb, pv->tag);
 	if (pv->flag & BANS_FLAG_HTTP)
-		ban_parse_http(b, a1 + strlen(pv->name));
+		ban_parse_http(bp, a1 + strlen(pv->name));
 
-	ban_add_lump(b, a3, strlen(a3) + 1);
+	ban_add_lump(bp, a3, strlen(a3) + 1);
 	if (!strcmp(a2, "~")) {
-		VSB_putc(b->vsb, BANS_OPER_MATCH);
-		i = ban_parse_regexp(b, a3);
-		if (i)
-			return (i);
+		VSB_putc(bp->vsb, BANS_OPER_MATCH);
+		err = ban_parse_regexp(bp, a3);
+		if (err)
+			return (err);
 	} else if (!strcmp(a2, "!~")) {
-		VSB_putc(b->vsb, BANS_OPER_NMATCH);
-		i = ban_parse_regexp(b, a3);
-		if (i)
-			return (i);
+		VSB_putc(bp->vsb, BANS_OPER_NMATCH);
+		err = ban_parse_regexp(bp, a3);
+		if (err)
+			return (err);
 	} else if (!strcmp(a2, "==")) {
-		VSB_putc(b->vsb, BANS_OPER_EQ);
+		VSB_putc(bp->vsb, BANS_OPER_EQ);
 	} else if (!strcmp(a2, "!=")) {
-		VSB_putc(b->vsb, BANS_OPER_NEQ);
+		VSB_putc(bp->vsb, BANS_OPER_NEQ);
 	} else {
-		return (ban_error(b,
+		return (ban_error(bp,
 		    "expected conditional (~, !~, == or !=) got \"%s\"", a2));
 	}
-	return (0);
+	return (NULL);
 }
 
 /*--------------------------------------------------------------------
@@ -197,121 +249,80 @@ BAN_AddTest(struct ban *b, const char *a1, const char *a2, const char *a3)
  *      deleted.
  */
 
-static char ban_error_nomem[] = "Could not get memory";
-
-static char *
-ban_ins_error(const char *p)
+const char *
+BAN_Commit(struct ban_proto *bp)
 {
-	char *r = NULL;
-
-	if (p != NULL)
-		r = strdup(p);
-	if (r == NULL)
-		r = ban_error_nomem;
-	return (r);
-}
-
-void
-BAN_Free_Errormsg(char *p)
-{
-	if (p != ban_error_nomem)
-		free(p);
-}
-
-char *
-BAN_Insert(struct ban *b)
-{
-	struct ban  *bi, *be;
+	struct ban  *b, *bi;
 	ssize_t ln;
 	double t0;
-	char *p;
 
-	CHECK_OBJ_NOTNULL(b, BAN_MAGIC);
-	AN(b->vsb);
+	CHECK_OBJ_NOTNULL(bp, BAN_PROTO_MAGIC);
+	AN(bp->vsb);
 
-	if (ban_shutdown) {
-		BAN_Free(b);
-		return (ban_ins_error("Shutting down"));
-	}
+	if (ban_shutdown)
+		return (ban_error(bp, "Shutting down"));
 
-	AZ(VSB_finish(b->vsb));
-	ln = VSB_len(b->vsb);
+	AZ(VSB_finish(bp->vsb));
+	ln = VSB_len(bp->vsb);
 	assert(ln >= 0);
 
-	if (b->flags & BANS_FLAG_ERROR) {
-		p = ban_ins_error(VSB_data(b->vsb));
-		BAN_Free(b);
-		return (p);
-	}
+	ALLOC_OBJ(b, BAN_MAGIC);
+	if (b == NULL)
+		return (ban_error(bp, ban_build_err_no_mem));
+	VTAILQ_INIT(&b->objcore);
 
 	b->spec = malloc(ln + BANS_HEAD_LEN);
 	if (b->spec == NULL) {
-		BAN_Free(b);
-		return (ban_ins_error(NULL));
+		free(b);
+		return (ban_error(bp, ban_build_err_no_mem));
 	}
 
+	b->flags = bp->flags;
+
 	memset(b->spec, 0, BANS_HEAD_LEN);
 	t0 = VTIM_real();
 	memcpy(b->spec + BANS_TIMESTAMP, &t0, sizeof t0);
 	b->spec[BANS_FLAGS] = b->flags & 0xff;
-	memcpy(b->spec + BANS_HEAD_LEN, VSB_data(b->vsb), ln);
+	memcpy(b->spec + BANS_HEAD_LEN, VSB_data(bp->vsb), ln);
 	ln += BANS_HEAD_LEN;
 	vbe32enc(b->spec + BANS_LENGTH, ln);
 
-	VSB_delete(b->vsb);
-	b->vsb = NULL;
-
 	Lck_Lock(&ban_mtx);
 	if (ban_shutdown) {
-		/* Check again, we might have raced */
+		/* We could have raced a shutdown */
 		Lck_Unlock(&ban_mtx);
 		BAN_Free(b);
-		return (ban_ins_error("Shutting down"));
+		return (ban_error(bp, "Shutting down"));
 	}
+	bi = VTAILQ_FIRST(&ban_head);
 	VTAILQ_INSERT_HEAD(&ban_head, b, list);
 	ban_start = b;
+
 	VSC_C_main->bans++;
 	VSC_C_main->bans_added++;
+	VSC_C_main->bans_persisted_bytes += ln;
+
 	if (b->flags & BANS_FLAG_OBJ)
 		VSC_C_main->bans_obj++;
 	if (b->flags & BANS_FLAG_REQ)
 		VSC_C_main->bans_req++;
 
-	be = VTAILQ_LAST(&ban_head, banhead_s);
-	if (cache_param->ban_dups && be != b)
-		be->refcount++;
-	else
-		be = NULL;
-
-	/* ban_magic is magic, and needs to be inserted early to give
-	 * a handle to grab a ref on. We don't report it here as the
-	 * stevedores will not be opened and ready to accept it
-	 * yet. Instead it is reported on BAN_Compile, which is after
-	 * the stevedores has been opened, but before any new objects
-	 * can have entered the cache (thus no objects in the mean
-	 * time depending on ban_magic in the list) */
-	VSC_C_main->bans_persisted_bytes += ln;
-	if (b != ban_magic)
-		ban_info(BI_NEW, b->spec, ln); /* Notify stevedores */
-	Lck_Unlock(&ban_mtx);
-
-	if (be == NULL)
-		return (NULL);
-
-	/* Hunt down duplicates, and mark them as completed */
-	bi = b;
-	Lck_Lock(&ban_mtx);
-	while (!ban_shutdown && bi != be) {
-		bi = VTAILQ_NEXT(bi, list);
-		if (bi->flags & BANS_FLAG_COMPLETED)
-			continue;
-		if (!ban_equal(b->spec, bi->spec))
-			continue;
-		ban_mark_completed(bi);
-		VSC_C_main->bans_dups++;
+	if (bi != NULL)
+		ban_info(BI_NEW, b->spec, ln);	/* Notify stevedores */
+
+	if (cache_param->ban_dups) {
+		/* Hunt down duplicates, and mark them as completed */
+		for (bi = VTAILQ_NEXT(b, list); bi != NULL;
+		    bi = VTAILQ_NEXT(bi, list)) {
+			if (!(bi->flags & BANS_FLAG_COMPLETED) &&
+			    ban_equal(b->spec, bi->spec)) {
+				ban_mark_completed(bi);
+				VSC_C_main->bans_dups++;
+			}
+		}
 	}
-	be->refcount--;
 	Lck_Unlock(&ban_mtx);
 
+	BAN_Abandon(bp);
 	return (NULL);
 }
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 642e754..09441e0 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -395,15 +395,16 @@ VRT_ban_string(VRT_CTX, const char *str)
 {
 	char *a1, *a2, *a3;
 	char **av;
-	struct ban *b;
+	struct ban_proto *bp;
+	const char *err;
 	int i;
 
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 	AN(ctx->vsl);
 	AN(str);
 
-	b = BAN_New();
-	if (b == NULL) {
+	bp = BAN_Build();
+	if (bp == NULL) {
 		VSLb(ctx->vsl, SLT_VCL_Error, "ban(): Out of Memory");
 		return;
 	}
@@ -412,7 +413,7 @@ VRT_ban_string(VRT_CTX, const char *str)
 	if (av[0] != NULL) {
 		VSLb(ctx->vsl, SLT_VCL_Error, "ban(): %s", av[0]);
 		VAV_Free(av);
-		BAN_Free(b);
+		BAN_Abandon(bp);
 		return;
 	}
 	for (i = 0; ;) {
@@ -434,13 +435,17 @@ VRT_ban_string(VRT_CTX, const char *str)
 			    "ban(): Expected second operand.");
 			break;
 		}
-		if (BAN_AddTest(b, a1, a2, a3) || av[++i] == NULL) {
-			a1 = BAN_Insert(b);
-			if (a1 != NULL) {
-				VSLb(ctx->vsl, SLT_VCL_Error,
-				    "ban(): %s", a1);
-				BAN_Free_Errormsg(a1);
-			}
+		err = BAN_AddTest(bp, a1, a2, a3);
+		if (err) {
+			VSLb(ctx->vsl, SLT_VCL_Error, "ban(): %s", err);
+			break;
+		}
+		if (av[++i] == NULL) {
+			err = BAN_Commit(bp);
+			if (err == NULL)
+				bp = NULL;
+			else
+				VSLb(ctx->vsl, SLT_VCL_Error, "ban(): %s", err);
 			break;
 		}
 		if (strcmp(av[i], "&&")) {
@@ -450,6 +455,8 @@ VRT_ban_string(VRT_CTX, const char *str)
 			break;
 		}
 	}
+	if (bp != NULL)
+		BAN_Abandon(bp);
 	VAV_Free(av);
 }
 
diff --git a/bin/varnishd/flint.lnt b/bin/varnishd/flint.lnt
index 5904b1e..38453d1 100644
--- a/bin/varnishd/flint.lnt
+++ b/bin/varnishd/flint.lnt
@@ -106,6 +106,7 @@
 
 -emacro(527, NEEDLESS_RETURN)	// unreachable code
 
+-sem(BAN_Free, custodial(1))
 -sem(EXP_Inject, custodial(1))
 -sem(HSH_Insert, custodial(3))
 -sem(WS_Init, custodial(2))



More information about the varnish-commit mailing list