[master] 55dffc358 Refactor the panic structure formatting code

Poul-Henning Kamp phk at FreeBSD.org
Thu Dec 3 14:50:07 UTC 2020


commit 55dffc3585685f2a67e3228094aea9ca9d986798
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Thu Dec 3 14:48:43 2020 +0000

    Refactor the panic structure formatting code
    
    This introduces a `PAN_dump_struct` function to do the
    standard up-front checks for NULL, bad magic & already dumped.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 10bd3079b..c7825f8fa 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -848,10 +848,9 @@ Tlen(const txt t)
  */
 #define W_TIM_real(w) ((w)->lastused = VTIM_real())
 
-#define PAN_CheckMagic(vsb, ptr, exp)					\
-	do {								\
-		if ((ptr)->magic != (exp))				\
-			VSB_printf((vsb),				\
-			    "MAGIC at %p is 0x%08x (Should be: %s/0x%08x)\n", \
-			    ptr, (ptr)->magic, #exp, exp);		\
-	} while(0)
+int PAN_dump_struct2(struct vsb *vsb, const void *ptr,
+    const char *smagic, unsigned magic, const char *fmt, ...)
+    v_printflike_(5,6);
+
+#define PAN_dump_struct(vsb, ptr, magic, ...) \
+    PAN_dump_struct2(vsb, ptr, #magic, magic, __VA_ARGS__)
diff --git a/bin/varnishd/cache/cache_deliver_proc.c b/bin/varnishd/cache/cache_deliver_proc.c
index 85d294b2d..7dbe03a0a 100644
--- a/bin/varnishd/cache/cache_deliver_proc.c
+++ b/bin/varnishd/cache/cache_deliver_proc.c
@@ -39,9 +39,8 @@ VDP_Panic(struct vsb *vsb, const struct vdp_ctx *vdc)
 {
 	struct vdp_entry *vde;
 
-	VSB_printf(vsb, "vdc = %p {\n", vdc);
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, vdc, VDP_CTX_MAGIC);
+	if (PAN_dump_struct(vsb, vdc, VDP_CTX_MAGIC, "vdc"))
+		return;
 	VSB_printf(vsb, "nxt = %p,\n", vdc->nxt);
 	VSB_printf(vsb, "retval = %d,\n", vdc->retval);
 
diff --git a/bin/varnishd/cache/cache_panic.c b/bin/varnishd/cache/cache_panic.c
index 84b4a77c8..e20e07f80 100644
--- a/bin/varnishd/cache/cache_panic.c
+++ b/bin/varnishd/cache/cache_panic.c
@@ -110,23 +110,39 @@ static const void *already_list[N_ALREADY];
 static int already_idx;
 
 int
-PAN_already(struct vsb *vsb, const void *ptr)
+PAN_dump_struct2(struct vsb *vsb, const void *ptr,
+    const char *smagic, unsigned magic, const char *fmt, ...)
 {
+	va_list ap;
+	const unsigned *uptr;
 	int i;
 
+	AN(vsb);
+	va_start(ap, fmt);
+	VSB_vprintf(vsb, fmt, ap);
+	va_end(ap);
 	if (ptr == NULL) {
-		VSB_cat(vsb, "},\n");
-		return (1);
+		VSB_printf(vsb, " = NULL\n");
+		return (-1);
 	}
+	VSB_printf(vsb, " = %p {\n", ptr);
 	for (i = 0; i < already_idx; i++) {
 		if (already_list[i] == ptr) {
 			VSB_cat(vsb, "  [Already dumped, see above]\n");
 			VSB_cat(vsb, "},\n");
-			return (1);
+			return (-2);
 		}
 	}
 	if (already_idx < N_ALREADY)
 		already_list[already_idx++] = ptr;
+	uptr = ptr;
+	if (*uptr != magic) {
+		VSB_printf(vsb, "  .magic = 0x%08x", *uptr);
+		VSB_printf(vsb, " EXPECTED: %s=0x%08x\n", smagic, magic);
+		VSB_printf(vsb, "}\n");
+		return (-3);
+	}
+	VSB_indent(vsb, 2);
 	return (0);
 }
 
@@ -136,15 +152,12 @@ static void
 pan_htc(struct vsb *vsb, const struct http_conn *htc)
 {
 
-	VSB_printf(vsb, "http_conn = %p {\n", htc);
-	if (PAN_already(vsb, htc))
+	if (PAN_dump_struct(vsb, htc, HTTP_CONN_MAGIC, "http_conn"))
 		return;
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, htc, HTTP_CONN_MAGIC);
 	if (htc->rfd != NULL)
 		VSB_printf(vsb, "fd = %d (@%p),\n", *htc->rfd, htc->rfd);
 	VSB_printf(vsb, "doclose = %s,\n", sess_close_2str(htc->doclose, 0));
-	WS_Panic(htc->ws, vsb);
+	WS_Panic(vsb, htc->ws);
 	VSB_printf(vsb, "{rxbuf_b, rxbuf_e} = {%p, %p},\n",
 	    htc->rxbuf_b, htc->rxbuf_e);
 	VSB_printf(vsb, "{pipeline_b, pipeline_e} = {%p, %p},\n",
@@ -168,12 +181,9 @@ pan_http(struct vsb *vsb, const char *id, const struct http *h)
 {
 	int i;
 
-	VSB_printf(vsb, "http[%s] = %p {\n", id, h);
-	if (PAN_already(vsb, h))
+	if (PAN_dump_struct(vsb, h, HTTP_MAGIC, "http[%s]", id))
 		return;
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, h, HTTP_MAGIC);
-	WS_Panic(h->ws, vsb);
+	WS_Panic(vsb, h->ws);
 	VSB_cat(vsb, "hdrs {\n");
 	VSB_indent(vsb, 2);
 	for (i = 0; i < h->nhd; ++i) {
@@ -189,14 +199,12 @@ pan_http(struct vsb *vsb, const char *id, const struct http *h)
 }
 
 /*--------------------------------------------------------------------*/
+
 static void
 pan_boc(struct vsb *vsb, const struct boc *boc)
 {
-	VSB_printf(vsb, "boc = %p {\n", boc);
-	if (PAN_already(vsb, boc))
+	if (PAN_dump_struct(vsb, boc, BOC_MAGIC, "boc"))
 		return;
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, boc, BOC_MAGIC);
 	VSB_printf(vsb, "refcnt = %u,\n", boc->refcount);
 	VSB_printf(vsb, "state = %s,\n", boc_state_2str(boc->state));
 	VSB_printf(vsb, "vary = %p,\n", boc->vary);
@@ -212,11 +220,8 @@ pan_objcore(struct vsb *vsb, const char *typ, const struct objcore *oc)
 {
 	const char *p;
 
-	VSB_printf(vsb, "objcore[%s] = %p {\n", typ, oc);
-	if (PAN_already(vsb, oc))
+	if (PAN_dump_struct(vsb, oc, OBJCORE_MAGIC, "objcore[%s]", typ))
 		return;
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, oc, OBJCORE_MAGIC);
 	VSB_printf(vsb, "refcnt = %d,\n", oc->refcnt);
 	VSB_cat(vsb, "flags = {");
 
@@ -267,12 +272,9 @@ pan_wrk(struct vsb *vsb, const struct worker *wrk)
 	unsigned m, u;
 	const char *p;
 
-	VSB_printf(vsb, "worker = %p {\n", wrk);
-	if (PAN_already(vsb, wrk))
+	if (PAN_dump_struct(vsb, wrk, WORKER_MAGIC, "worker"))
 		return;
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, wrk, WORKER_MAGIC);
-	WS_Panic(wrk->aws, vsb);
+	WS_Panic(vsb, wrk->aws);
 
 	m = wrk->cur_method;
 	VSB_cat(vsb, "VCL::method = ");
@@ -314,9 +316,8 @@ pan_vfp(struct vsb *vsb, const struct vfp_ctx *vfc)
 {
 	struct vfp_entry *vfe;
 
-	VSB_printf(vsb, "vfc = %p {\n", vfc);
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, vfc, VFP_CTX_MAGIC);
+	if (PAN_dump_struct(vsb, vfc, VFP_CTX_MAGIC, "vfc"))
+		return;
 	VSB_printf(vsb, "failed = %d,\n", vfc->failed);
 	VSB_printf(vsb, "req = %p,\n", vfc->req);
 	VSB_printf(vsb, "resp = %p,\n", vfc->resp);
@@ -349,11 +350,8 @@ pan_busyobj(struct vsb *vsb, const struct busyobj *bo)
 {
 	const char *p;
 
-	VSB_printf(vsb, "busyobj = %p {\n", bo);
-	if (PAN_already(vsb, bo))
+	if (PAN_dump_struct(vsb, bo, BUSYOBJ_MAGIC, "busyobj"))
 		return;
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, bo, BUSYOBJ_MAGIC);
 	VSB_printf(vsb, "end = %p,\n", bo->end);
 	VSB_printf(vsb, "retries = %u,\n", bo->retries);
 
@@ -369,7 +367,7 @@ pan_busyobj(struct vsb *vsb, const struct busyobj *bo)
 	if (bo->vfp_filter_list != NULL)
 		VSB_printf(vsb, "filter_list = \"%s\",\n", bo->vfp_filter_list);
 
-	WS_Panic(bo->ws, vsb);
+	WS_Panic(vsb, bo->ws);
 	VSB_printf(vsb, "ws_bo = %p,\n", (void *)bo->ws_bo);
 
 	// bereq0 left out
@@ -413,10 +411,8 @@ pan_busyobj(struct vsb *vsb, const struct busyobj *bo)
 static void
 pan_top(struct vsb *vsb, const struct reqtop *top)
 {
-	VSB_printf(vsb, "top = %p {\n", top);
-	if (PAN_already(vsb, top))
+	if (PAN_dump_struct(vsb, top, REQ_MAGIC, "top"))
 		return;
-	VSB_indent(vsb, 2);
 	pan_req(vsb, top->topreq);
 	pan_privs(vsb, top->privs);
 	VCL_Panic(vsb, "vcl0", top->vcl0);
@@ -431,11 +427,8 @@ pan_req(struct vsb *vsb, const struct req *req)
 {
 	const struct transport *xp;
 
-	VSB_printf(vsb, "req = %p {\n", req);
-	if (PAN_already(vsb, req))
+	if (PAN_dump_struct(vsb, req, REQ_MAGIC, "req"))
 		return;
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, req, REQ_MAGIC);
 	xp = req->transport;
 	VSB_printf(vsb, "vxid = %u, transport = %s", VXID(req->vsl->wid),
 	    xp == NULL ? "NULL" : xp->name);
@@ -470,7 +463,7 @@ pan_req(struct vsb *vsb, const struct req *req)
 	if (req->wrk != NULL)
 		pan_wrk(vsb, req->wrk);
 
-	WS_Panic(req->ws, vsb);
+	WS_Panic(vsb, req->ws);
 	if (VALID_OBJ(req->htc, HTTP_CONN_MAGIC))
 		pan_htc(vsb, req->htc);
 	pan_http(vsb, "req", req->http);
@@ -523,11 +516,8 @@ pan_sess(struct vsb *vsb, const struct sess *sp)
 	const char *cp;
 	const struct transport *xp;
 
-	VSB_printf(vsb, "sp = %p {\n", sp);
-	if (PAN_already(vsb, sp))
+	if (PAN_dump_struct(vsb, sp, SESS_MAGIC, "sess"))
 		return;
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, sp, SESS_MAGIC);
 	VSB_printf(vsb, "fd = %d, vxid = %u,\n",
 	    sp->fd, VXID(sp->vxid));
 	VSB_printf(vsb, "t_open = %f,\n", sp->t_open);
@@ -539,7 +529,7 @@ pan_sess(struct vsb *vsb, const struct sess *sp)
 		return;
 	}
 
-	WS_Panic(sp->ws, vsb);
+	WS_Panic(vsb, sp->ws);
 	xp = XPORT_ByNumber(sp->sattr[SA_TRANSPORT]);
 	VSB_printf(vsb, "transport = %s",
 	    xp == NULL ? "<none>" : xp->name);
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index 1fc5e41f7..84316cee8 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -350,7 +350,6 @@ void ObjUnsubscribeEvents(uintptr_t *);
 
 /* cache_panic.c */
 void PAN_Init(void);
-int PAN_already(struct vsb *, const void *);
 const char *sess_close_2str(enum sess_close sc, int want_desc);
 
 /* cache_pool.c */
@@ -481,7 +480,7 @@ void WRK_Init(void);
 
 /* cache_ws.c */
 void WS_Id(const struct ws *ws, char *id);
-void WS_Panic(const struct ws *ws, struct vsb *vsb);
+void WS_Panic(struct vsb *, const struct ws *);
 static inline int
 WS_IsReserved(const struct ws *ws)
 {
diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index 6c7427eb6..f0b035b7b 100644
--- a/bin/varnishd/cache/cache_vcl.c
+++ b/bin/varnishd/cache/cache_vcl.c
@@ -251,48 +251,48 @@ vcl_find(const char *name)
 
 /*--------------------------------------------------------------------*/
 
+static void
+vcl_panic_conf(struct vsb *vsb, const struct VCL_conf *conf)
+{
+	int i;
+	const struct vpi_ii *ii;
+
+	if (PAN_dump_struct(vsb, conf, VCL_CONF_MAGIC, "conf"))
+		return;
+	VSB_printf(vsb, "syntax = \"%u\",\n", conf->syntax);
+	VSB_cat(vsb, "srcname = {\n");
+	VSB_indent(vsb, 2);
+	for (i = 0; i < conf->nsrc; ++i)
+		VSB_printf(vsb, "\"%s\",\n", conf->srcname[i]);
+	VSB_indent(vsb, -2);
+	VSB_cat(vsb, "},\n");
+	VSB_cat(vsb, "instances = {\n");
+	VSB_indent(vsb, 2);
+	ii = conf->instance_info;
+	while (ii != NULL && ii->p != NULL) {
+		VSB_printf(vsb, "\"%s\" = %p,\n", ii->name,
+		    (const void *)*(const uintptr_t *)ii->p);
+		ii++;
+	}
+	VSB_indent(vsb, -2);
+	VSB_cat(vsb, "},\n");
+	VSB_indent(vsb, -2);
+	VSB_cat(vsb, "},\n");
+}
+
 void
 VCL_Panic(struct vsb *vsb, const char *nm, const struct vcl *vcl)
 {
-	const struct vpi_ii *ii;
-	int i;
 
 	AN(vsb);
-	if (vcl == NULL)
+	if (PAN_dump_struct(vsb, vcl, VCL_MAGIC, "vcl[%s]", nm))
 		return;
-	VSB_printf(vsb, "%s = {\n", nm);
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, vcl, VCL_MAGIC);
 	VSB_printf(vsb, "name = \"%s\",\n", vcl->loaded_name);
 	VSB_printf(vsb, "busy = %u,\n", vcl->busy);
 	VSB_printf(vsb, "discard = %u,\n", vcl->discard);
 	VSB_printf(vsb, "state = %s,\n", vcl->state);
 	VSB_printf(vsb, "temp = %s,\n", vcl->temp ? vcl->temp->name : "(null)");
-	VSB_cat(vsb, "conf = {\n");
-	VSB_indent(vsb, 2);
-	if (vcl->conf == NULL) {
-		VSB_cat(vsb, "conf = NULL\n");
-	} else {
-		PAN_CheckMagic(vsb, vcl->conf, VCL_CONF_MAGIC);
-		VSB_printf(vsb, "syntax = \"%u\",\n", vcl->conf->syntax);
-		VSB_cat(vsb, "srcname = {\n");
-		VSB_indent(vsb, 2);
-		for (i = 0; i < vcl->conf->nsrc; ++i)
-			VSB_printf(vsb, "\"%s\",\n", vcl->conf->srcname[i]);
-		VSB_indent(vsb, -2);
-		VSB_cat(vsb, "instances = {\n");
-		VSB_indent(vsb, 2);
-		ii = vcl->conf->instance_info;
-		while (ii != NULL && ii->p != NULL) {
-			VSB_printf(vsb, "\"%s\" = %p,\n", ii->name,
-			    (const void *)*(const uintptr_t *)ii->p);
-			ii++;
-		}
-		VSB_indent(vsb, -2);
-		VSB_cat(vsb, "},\n");
-	}
-	VSB_indent(vsb, -2);
-	VSB_cat(vsb, "},\n");
+	vcl_panic_conf(vsb, vcl->conf);
 	VSB_indent(vsb, -2);
 	VSB_cat(vsb, "},\n");
 }
diff --git a/bin/varnishd/cache/cache_vrt_priv.c b/bin/varnishd/cache/cache_vrt_priv.c
index 53183489f..fa53089f0 100644
--- a/bin/varnishd/cache/cache_vrt_priv.c
+++ b/bin/varnishd/cache/cache_vrt_priv.c
@@ -64,35 +64,35 @@ pan_privs(struct vsb *vsb, const struct vrt_privs *privs)
 	const struct vmod_priv *p;
 	const struct vmod_priv_methods *m;
 
-	VSB_printf(vsb, "privs = %p {\n", privs);
-	if (PAN_already(vsb, privs))
+	if (privs == NULL) {
+		VSB_printf(vsb, "privs = NULL\n");
 		return;
+	}
+	VSB_printf(vsb, "privs = %p {\n", privs);
 	VSB_indent(vsb, 2);
-	if (privs != NULL) {
-		VRBT_FOREACH(vp, vrt_privs, privs) {
-			PAN_CheckMagic(vsb, vp, VRT_PRIV_MAGIC);
-			p = vp->priv;
-			//lint -e{774}
-			if (p == NULL) {
-				// should never happen
-				VSB_printf(vsb, "priv NULL vmod %jx\n",
-				    (uintmax_t)vp->vmod_id);
-				continue;
-			}
-			m = p->methods;
-			VSB_printf(vsb,
-			    "priv {p %p l %ld m %p t \"%s\"} vmod %jx\n",
-			    p->priv, p->len, m,
-			    m != NULL ? m->type : "",
-			    (uintmax_t)vp->vmod_id
-			);
+	VRBT_FOREACH(vp, vrt_privs, privs) {
+		if (PAN_dump_struct(vsb, vp, VRT_PRIV_MAGIC, "priv"))
+			continue;
+		p = vp->priv;
+		//lint -e{774}
+		if (p == NULL) {
+			// should never happen
+			VSB_printf(vsb, "priv NULL vmod %jx\n",
+			    (uintmax_t)vp->vmod_id);
+			continue;
 		}
+		m = p->methods;
+		VSB_printf(vsb,
+		    "priv {p %p l %ld m %p t \"%s\"} vmod %jx\n",
+		    p->priv, p->len, m,
+		    m != NULL ? m->type : "",
+		    (uintmax_t)vp->vmod_id
+		);
 	}
 	VSB_indent(vsb, -2);
 	VSB_cat(vsb, "},\n");
 }
 
-
 /*--------------------------------------------------------------------
  */
 
diff --git a/bin/varnishd/cache/cache_ws.c b/bin/varnishd/cache/cache_ws.c
index 36edfd68e..d1f54a2fd 100644
--- a/bin/varnishd/cache/cache_ws.c
+++ b/bin/varnishd/cache/cache_ws.c
@@ -430,14 +430,11 @@ WS_VSB_finish(struct vsb *vsb, struct ws *ws, size_t *szp)
 /*--------------------------------------------------------------------*/
 
 void
-WS_Panic(const struct ws *ws, struct vsb *vsb)
+WS_Panic(struct vsb *vsb, const struct ws *ws)
 {
 
-	VSB_printf(vsb, "ws = %p {\n", ws);
-	if (PAN_already(vsb, ws))
+	if (PAN_dump_struct(vsb, ws, WS_MAGIC, "ws"))
 		return;
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, ws, WS_MAGIC);
 	if (ws->id[0] != '\0' && (!(ws->id[0] & 0x20)))	// cheesy islower()
 		VSB_cat(vsb, "OVERFLOWED ");
 	VSB_printf(vsb, "id = \"%s\",\n", ws->id);
diff --git a/bin/varnishd/http2/cache_http2_panic.c b/bin/varnishd/http2/cache_http2_panic.c
index ddc4eb761..e2a3e1e0a 100644
--- a/bin/varnishd/http2/cache_http2_panic.c
+++ b/bin/varnishd/http2/cache_http2_panic.c
@@ -45,12 +45,14 @@ h2_sess_panic(struct vsb *vsb, const struct sess *sp)
 
 	AZ(SES_Get_proto_priv(sp, &up));
 
+	AN(up);
+	h2 = (void*)*up;
+	if (PAN_dump_struct(vsb, h2, H2_SESS_MAGIC, "h2_sess"))
+		return;
 	h2 = (void*)*up;
-	CHECK_OBJ_NOTNULL(h2, H2_SESS_MAGIC);
-	VSB_cat(vsb, "streams {\n");
-	VSB_indent(vsb, 2);
 	VTAILQ_FOREACH(r2, &h2->streams, list) {
-		PAN_CheckMagic(vsb, r2, H2_REQ_MAGIC);
+		if (PAN_dump_struct(vsb, r2, H2_REQ_MAGIC, "stream"))
+			continue;
 		VSB_printf(vsb, "0x%08x", r2->stream);
 		switch (r2->state) {
 #define H2_STREAM(U,sd,d) case H2_S_##U: VSB_printf(vsb, " %-6s", sd); break;
@@ -59,8 +61,9 @@ h2_sess_panic(struct vsb *vsb, const struct sess *sp)
 			VSB_printf(vsb, " State %d", r2->state);
 			break;
 		}
-		VSB_cat(vsb, "\n");
+		VSB_cat(vsb, "},\n");
+		VSB_indent(vsb, -2);
 	}
+	VSB_cat(vsb, "},\n");
 	VSB_indent(vsb, -2);
-	VSB_cat(vsb, "}\n");
 }


More information about the varnish-commit mailing list