[master] 7f2888877 Wrap vxid in a protective struct for type-consistency.

Poul-Henning Kamp phk at FreeBSD.org
Mon Oct 10 09:40:08 UTC 2022


commit 7f28888779fd14f99eb34e50f6fb07ea6bbff999
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Oct 10 09:05:32 2022 +0000

    Wrap vxid in a protective struct for type-consistency.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 450772696..57e243fb8 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -57,6 +57,20 @@
 
 /*--------------------------------------------------------------------*/
 
+struct vxids {
+	uint32_t	vxid;
+};
+
+typedef struct vxids vxid_t;
+
+#define NO_VXID ((struct vxids){0})
+#define IS_NO_VXID(x) ((x).vxid == 0)
+#define VXID_TAG(x) ((x).vxid & (VSL_CLIENTMARKER|VSL_BACKENDMARKER))
+#define VXID(u) ((u.vxid) & VSL_IDENTMASK)
+#define IS_SAME_VXID(x, y) ((x).vxid == (y).vxid)
+
+/*--------------------------------------------------------------------*/
+
 struct body_status {
 	const char		*name;
 	int			nbr;
@@ -179,9 +193,9 @@ struct acct_bereq {
 /*--------------------------------------------------------------------*/
 
 struct vsl_log {
-	uint32_t                *wlb, *wlp, *wle;
-	unsigned                wlr;
-	unsigned                wid;
+	uint32_t		*wlb, *wlp, *wle;
+	unsigned		wlr;
+	vxid_t			wid;
 };
 
 /*--------------------------------------------------------------------*/
@@ -557,7 +571,7 @@ struct sess {
 	struct listen_sock	*listen_sock;
 	int			refcnt;
 	int			fd;
-	uint32_t		vxid;
+	vxid_t			vxid;
 
 	struct lock		mtx;
 
@@ -690,9 +704,7 @@ extern const char H__Reason[];
 #define http_range_at(str, tok, l)	http_tok_at(str, #tok, l)
 
 /* cache_main.c */
-#define NO_VXID (0U)
-#define VXID(u) ((u) & VSL_IDENTMASK)
-uint32_t VXID_Get(const struct worker *, uint32_t marker);
+vxid_t VXID_Get(const struct worker *, uint32_t marker);
 extern pthread_key_t witness_key;
 
 /* cache_lck.c */
@@ -761,10 +773,10 @@ ssize_t VRB_Iterate(struct worker *, struct vsl_log *, struct req *,
 const char *SES_Get_String_Attr(const struct sess *sp, enum sess_attr a);
 
 /* cache_shmlog.c */
-void VSLv(enum VSL_tag_e tag, uint32_t vxid, const char *fmt, va_list va);
-void VSL(enum VSL_tag_e tag, uint32_t vxid, const char *fmt, ...)
+void VSLv(enum VSL_tag_e tag, vxid_t vxid, const char *fmt, va_list va);
+void VSL(enum VSL_tag_e tag, vxid_t vxid, const char *fmt, ...)
     v_printflike_(3, 4);
-void VSLs(enum VSL_tag_e tag, uint32_t vxid, const struct strands *s);
+void VSLs(enum VSL_tag_e tag, vxid_t vxid, const struct strands *s);
 void VSLbv(struct vsl_log *, enum VSL_tag_e tag, const char *fmt, va_list va);
 void VSLb(struct vsl_log *, enum VSL_tag_e tag, const char *fmt, ...)
     v_printflike_(3, 4);
diff --git a/bin/varnishd/cache/cache_acceptor.c b/bin/varnishd/cache/cache_acceptor.c
index afdfbf222..67947114b 100644
--- a/bin/varnishd/cache/cache_acceptor.c
+++ b/bin/varnishd/cache/cache_acceptor.c
@@ -269,7 +269,7 @@ vca_sock_opt_set(const struct listen_sock *ls, const struct sess *sp)
 {
 	struct conn_heritage *ch;
 	struct sock_opt *so;
-	unsigned vxid;
+	vxid_t vxid;
 	int n, sock;
 
 	CHECK_OBJ_NOTNULL(ls, LISTEN_SOCK_MAGIC);
@@ -280,7 +280,7 @@ vca_sock_opt_set(const struct listen_sock *ls, const struct sess *sp)
 		vxid = sp->vxid;
 	} else {
 		sock = ls->sock;
-		vxid = 0;
+		vxid = NO_VXID;
 	}
 
 	for (n = 0; n < n_sock_opts; n++) {
@@ -559,7 +559,7 @@ vca_accept_task(struct worker *wrk, void *arg)
 				    lport, VTCP_PORTBUFSIZE);
 			}
 
-			VSL(SLT_SessError, 0, "%s %s %s %d %d \"%s\"",
+			VSL(SLT_SessError, NO_VXID, "%s %s %s %d %d \"%s\"",
 			    wa.acceptlsock->name, laddr, lport,
 			    ls->sock, i, VAS_errtxt(i));
 			(void)Pool_TrySumstat(wrk);
@@ -587,7 +587,7 @@ vca_accept_task(struct worker *wrk, void *arg)
 
 	}
 
-	VSL(SLT_Debug, 0, "XXX Accept thread dies %p", ps);
+	VSL(SLT_Debug, NO_VXID, "XXX Accept thread dies %p", ps);
 	FREE_OBJ(ps);
 }
 
@@ -690,7 +690,7 @@ ccf_start(struct cli *cli, const char * const *av, void *priv)
 		assert (ls->sock > 0);	// We know where stdin is
 		if (cache_param->tcp_fastopen &&
 		    VTCP_fastopen(ls->sock, cache_param->listen_depth))
-			VSL(SLT_Error, 0,
+			VSL(SLT_Error, NO_VXID,
 			    "Kernel TCP Fast Open: sock=%d, errno=%d %s",
 			    ls->sock, errno, VAS_errtxt(errno));
 		if (listen(ls->sock, cache_param->listen_depth)) {
@@ -706,7 +706,7 @@ ccf_start(struct cli *cli, const char * const *av, void *priv)
 		ls->test_heritage = 1;
 		vca_sock_opt_set(ls, NULL);
 		if (cache_param->accept_filter && VTCP_filter_http(ls->sock))
-			VSL(SLT_Error, 0,
+			VSL(SLT_Error, NO_VXID,
 			    "Kernel filtering: sock=%d, errno=%d %s",
 			    ls->sock, errno, VAS_errtxt(errno));
 	}
diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 33827b916..5a7bba66c 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -133,7 +133,7 @@ ved_include(struct req *preq, const char *src, const char *host,
 	req = Req_New(sp);
 	AN(req);
 	THR_SetRequest(req);
-	AZ(req->vsl->wid);
+	assert(IS_NO_VXID(req->vsl->wid));
 	req->vsl->wid = VXID_Get(wrk, VSL_CLIENTMARKER);
 
 	wrk->stats->esi_req++;
diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c
index 6c9f1da6c..3e78feda6 100644
--- a/bin/varnishd/cache/cache_http.c
+++ b/bin/varnishd/cache/cache_http.c
@@ -211,7 +211,7 @@ http_VSLH(const struct http *hp, unsigned hdr)
 	int i;
 
 	if (hp->vsl != NULL) {
-		AN(hp->vsl->wid & (VSL_CLIENTMARKER|VSL_BACKENDMARKER));
+		assert(VXID_TAG(hp->vsl->wid));
 		i = hdr;
 		if (i > HTTP_HDR_FIRST)
 			i = HTTP_HDR_FIRST;
@@ -228,7 +228,7 @@ http_VSLH_del(const struct http *hp, unsigned hdr)
 	if (hp->vsl != NULL) {
 		/* We don't support unsetting stuff in the first line */
 		assert (hdr >= HTTP_HDR_FIRST);
-		AN(hp->vsl->wid & (VSL_CLIENTMARKER|VSL_BACKENDMARKER));
+		assert(VXID_TAG(hp->vsl->wid));
 		i = (HTTP_HDR_UNSET - HTTP_HDR_METHOD);
 		i += hp->logtag;
 		VSLbt(hp->vsl, (enum VSL_tag_e)i, hp->hd[hdr]);
diff --git a/bin/varnishd/cache/cache_main.c b/bin/varnishd/cache/cache_main.c
index b8a8174e1..ff6ef5c39 100644
--- a/bin/varnishd/cache/cache_main.c
+++ b/bin/varnishd/cache/cache_main.c
@@ -183,15 +183,16 @@ static uint32_t vxid_base;
 static uint32_t vxid_chunk = 32768;
 static struct lock vxid_lock;
 
-uint32_t
+vxid_t
 VXID_Get(const struct worker *wrk, uint32_t mask)
 {
 	struct vxid_pool *v;
+	vxid_t retval;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(wrk->wpriv, WORKER_PRIV_MAGIC);
 	v = wrk->wpriv->vxid_pool;
-	AZ(VXID(mask));
+	AZ(mask & VSL_IDENTMASK);
 	do {
 		if (v->count == 0) {
 			Lck_Lock(&vxid_lock);
@@ -203,7 +204,8 @@ VXID_Get(const struct worker *wrk, uint32_t mask)
 		v->count--;
 		v->next++;
 	} while (v->next == 0);
-	return (v->next | mask);
+	retval.vxid = v->next | mask;
+	return (retval);
 }
 
 /*--------------------------------------------------------------------
@@ -272,7 +274,7 @@ static struct cli_proto debug_cmds[] = {
 static void
 child_malloc_fail(void *p, const char *s)
 {
-	VSL(SLT_Error, 0, "MALLOC ERROR: %s (%p)", s, p);
+	VSL(SLT_Error, NO_VXID, "MALLOC ERROR: %s (%p)", s, p);
 	fprintf(stderr, "MALLOC ERROR: %s (%p)\n", s, p);
 	WRONG("Malloc Error");
 }
diff --git a/bin/varnishd/cache/cache_req.c b/bin/varnishd/cache/cache_req.c
index c5a394ef9..46166956d 100644
--- a/bin/varnishd/cache/cache_req.c
+++ b/bin/varnishd/cache/cache_req.c
@@ -55,7 +55,7 @@ Req_AcctLogCharge(struct VSC_main_wrk *ds, struct req *req)
 
 	a = &req->acct;
 
-	if (req->vsl->wid && !(req->res_mode & RES_PIPE)) {
+	if (!IS_NO_VXID(req->vsl->wid) && !(req->res_mode & RES_PIPE)) {
 		VSLb(req->vsl, SLT_ReqAcct, "%ju %ju %ju %ju %ju %ju",
 		    (uintmax_t)a->req_hdrbytes,
 		    (uintmax_t)a->req_bodybytes,
@@ -194,7 +194,7 @@ Req_Release(struct req *req)
 #include "tbl/acct_fields_req.h"
 
 	AZ(req->vcl);
-	if (req->vsl->wid)
+	if (!IS_NO_VXID(req->vsl->wid))
 		VSL_End(req->vsl);
 #ifdef ENABLE_WORKSPACE_EMULATOR
 	WS_Rollback(req->ws, 0);
@@ -262,12 +262,12 @@ Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req)
 		VCL_Recache(wrk, &req->vcl);
 
 	/* Charge and log byte counters */
-	if (req->vsl->wid) {
+	if (!IS_NO_VXID(req->vsl->wid)) {
 		Req_AcctLogCharge(wrk->stats, req);
-		if (req->vsl->wid != sp->vxid)
+		if (!IS_SAME_VXID(req->vsl->wid, sp->vxid))
 			VSL_End(req->vsl);
 		else
-			req->vsl->wid = 0; /* ending an h2 stream 0 */
+			req->vsl->wid = NO_VXID; /* ending an h2 stream 0 */
 	}
 
 	if (!isnan(req->t_prev) && req->t_prev > 0. && req->t_prev > sp->t_idle)
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 95c108789..1a1fd7e2a 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -1113,7 +1113,7 @@ cnt_diag(struct req *req, const char *state)
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 
 	VSLb(req->vsl,  SLT_Debug, "vxid %u STP_%s sp %p vcl %p",
-	    req->vsl->wid, state, req->sp, req->vcl);
+	    req->vsl->wid.vxid, state, req->sp, req->vcl);	// XXX_VXID
 	VSL_Flush(req->vsl, 0);
 }
 
@@ -1160,7 +1160,7 @@ CNT_Request(struct req *req)
 	    req->req_step == R_STP_LOOKUP ||
 	    req->req_step == R_STP_TRANSPORT);
 
-	AN(req->vsl->wid & VSL_CLIENTMARKER);
+	AN(VXID_TAG(req->vsl->wid) & VSL_CLIENTMARKER);
 	AN(req->vcl);
 
 	for (nxt = REQ_FSM_MORE; nxt == REQ_FSM_MORE; ) {
@@ -1192,7 +1192,7 @@ CNT_Request(struct req *req)
 				VCL_Recache(wrk, &req->top->vcl0);
 		}
 		VCL_TaskLeave(ctx, req->privs);
-		AN(req->vsl->wid);
+		assert(!IS_NO_VXID(req->vsl->wid));
 		VRB_Free(req);
 		VRT_Assign_Backend(&req->director_hint, NULL);
 		req->wrk = NULL;
diff --git a/bin/varnishd/cache/cache_shmlog.c b/bin/varnishd/cache/cache_shmlog.c
index ca7fc7d1d..8c2ca46c0 100644
--- a/bin/varnishd/cache/cache_shmlog.c
+++ b/bin/varnishd/cache/cache_shmlog.c
@@ -134,7 +134,7 @@ vsl_tag_is_masked(enum VSL_tag_e tag)
  */
 
 static inline uint32_t *
-vsl_hdr(enum VSL_tag_e tag, uint32_t *p, unsigned len, uint32_t vxid)
+vsl_hdr(enum VSL_tag_e tag, uint32_t *p, unsigned len, vxid_t vxid)
 {
 
 	AZ((uintptr_t)p & 0x3);
@@ -142,7 +142,7 @@ vsl_hdr(enum VSL_tag_e tag, uint32_t *p, unsigned len, uint32_t vxid)
 	assert(tag < SLT__Reserved);
 	AZ(len & ~VSL_LENMASK);
 
-	p[1] = vxid;
+	p[1] = vxid.vxid;
 	p[0] = ((((unsigned)tag & 0xff) << 24) | len);
 	return (VSL_END(p, len));
 }
@@ -227,7 +227,7 @@ vsl_get(unsigned len, unsigned records, unsigned flushes)
  */
 
 static void
-vslr(enum VSL_tag_e tag, uint32_t vxid, const char *b, unsigned len)
+vslr(enum VSL_tag_e tag, vxid_t vxid, const char *b, unsigned len)
 {
 	uint32_t *p;
 	unsigned mlen;
@@ -246,7 +246,7 @@ vslr(enum VSL_tag_e tag, uint32_t vxid, const char *b, unsigned len)
 	 * vsl_hdr() writes p[1] again, but we want to make sure it
 	 * has hit memory because we work on the live buffer here.
 	 */
-	p[1] = vxid;
+	p[1] = vxid.vxid;
 	VWMB();
 	(void)vsl_hdr(tag, p, len, vxid);
 }
@@ -259,7 +259,7 @@ vslr(enum VSL_tag_e tag, uint32_t vxid, const char *b, unsigned len)
  */
 
 void
-VSLv(enum VSL_tag_e tag, uint32_t vxid, const char *fmt, va_list ap)
+VSLv(enum VSL_tag_e tag, vxid_t vxid, const char *fmt, va_list ap)
 {
 	unsigned n, mlen = cache_param->vsl_reclen;
 	char buf[mlen];
@@ -280,7 +280,7 @@ VSLv(enum VSL_tag_e tag, uint32_t vxid, const char *fmt, va_list ap)
 }
 
 void
-VSLs(enum VSL_tag_e tag, uint32_t vxid, const struct strands *s)
+VSLs(enum VSL_tag_e tag, vxid_t vxid, const struct strands *s)
 {
 	unsigned n, mlen = cache_param->vsl_reclen;
 	char buf[mlen];
@@ -294,7 +294,7 @@ VSLs(enum VSL_tag_e tag, uint32_t vxid, const struct strands *s)
 }
 
 void
-VSL(enum VSL_tag_e tag, uint32_t vxid, const char *fmt, ...)
+VSL(enum VSL_tag_e tag, vxid_t vxid, const char *fmt, ...)
 {
 	va_list ap;
 
@@ -567,16 +567,16 @@ VSL_Setup(struct vsl_log *vsl, void *ptr, size_t len)
 	vsl->wle = ptr;
 	vsl->wle += len / sizeof(*vsl->wle);
 	vsl->wlr = 0;
-	vsl->wid = 0;
+	vsl->wid = NO_VXID;
 	vsl_sanity(vsl);
 }
 
 /*--------------------------------------------------------------------*/
 
 void
-VSL_ChgId(struct vsl_log *vsl, const char *typ, const char *why, uint32_t vxid)
+VSL_ChgId(struct vsl_log *vsl, const char *typ, const char *why, vxid_t vxid)
 {
-	uint32_t ovxid;
+	vxid_t ovxid;
 
 	vsl_sanity(vsl);
 	ovxid = vsl->wid;
@@ -595,12 +595,12 @@ VSL_End(struct vsl_log *vsl)
 	char p[] = "";
 
 	vsl_sanity(vsl);
-	AN(vsl->wid);
+	assert(!IS_NO_VXID(vsl->wid));
 	t.b = p;
 	t.e = p;
 	VSLbt(vsl, SLT_End, t);
 	VSL_Flush(vsl, 0);
-	vsl->wid = 0;
+	vsl->wid = NO_VXID;
 }
 
 static void v_matchproto_(vsm_lock_f)
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index 6d5adea5d..b40e5b43f 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -449,7 +449,7 @@ extern struct VSC_main *VSC_C_main;
 void VSM_Init(void);
 void VSL_Setup(struct vsl_log *vsl, void *ptr, size_t len);
 void VSL_ChgId(struct vsl_log *vsl, const char *typ, const char *why,
-    uint32_t vxid);
+    vxid_t vxid);
 void VSL_End(struct vsl_log *vsl);
 void VSL_Flush(struct vsl_log *, int overflow);
 
diff --git a/bin/varnishd/fuzzers/esi_parse_fuzzer.c b/bin/varnishd/fuzzers/esi_parse_fuzzer.c
index 8d2ff3caa..95b25c391 100644
--- a/bin/varnishd/fuzzers/esi_parse_fuzzer.c
+++ b/bin/varnishd/fuzzers/esi_parse_fuzzer.c
@@ -63,7 +63,7 @@ PAN__DumpStruct(struct vsb *vsb, int block, int track, const void *ptr,
 }
 
 void
-VSL(enum VSL_tag_e tag, uint32_t vxid, const char *fmt, ...)
+VSL(enum VSL_tag_e tag, vxid_t vxid, const char *fmt, ...)
 {
 	(void)tag;
 	(void)vxid;
diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c
index ab92dcf38..f22ffeef7 100644
--- a/bin/varnishd/http1/cache_http1_fsm.c
+++ b/bin/varnishd/http1/cache_http1_fsm.c
@@ -251,7 +251,7 @@ http1_dissect(struct worker *wrk, struct req *req)
 	CHECK_OBJ_NOTNULL(req->transport, TRANSPORT_MAGIC);
 
 	/* Allocate a new vxid now that we know we'll need it. */
-	AZ(req->vsl->wid);
+	assert(IS_NO_VXID(req->vsl->wid));
 	req->vsl->wid = VXID_Get(wrk, VSL_CLIENTMARKER);
 
 	VSLb(req->vsl, SLT_Begin, "req %u rxreq", VXID(req->sp->vxid));
diff --git a/vmod/vmod_vtc.c b/vmod/vmod_vtc.c
index 636bdc275..aa1c2b9ad 100644
--- a/vmod/vmod_vtc.c
+++ b/vmod/vmod_vtc.c
@@ -60,7 +60,7 @@ vmod_barrier_sync(VRT_CTX, VCL_STRING addr, VCL_DURATION tmo)
 	if (ctx->vsl != NULL)
 		VSLb(ctx->vsl, SLT_Debug, "barrier_sync(\"%s\")", addr);
 	else
-		VSL(SLT_Debug, 0, "barrier_sync(\"%s\")", addr);
+		VSL(SLT_Debug, NO_VXID, "barrier_sync(\"%s\")", addr);
 
 	sock = VTCP_open(addr, NULL, 0., &err);
 	if (sock < 0) {
@@ -409,7 +409,7 @@ VCL_VOID
 vmod_vsl(VRT_CTX, VCL_INT id, VCL_STRING tag_s, VCL_ENUM side, VCL_STRANDS s)
 {
 	struct vsl_tag2enum *te, key;
-	uint32_t vxid;
+	vxid_t vxid;
 
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 
@@ -427,11 +427,11 @@ vmod_vsl(VRT_CTX, VCL_INT id, VCL_STRING tag_s, VCL_ENUM side, VCL_STRANDS s)
 		return;
 	}
 
-	vxid = id & VSL_IDENTMASK;
+	vxid.vxid = id & VSL_IDENTMASK;
 	if (side == VENUM(c))
-		vxid |= VSL_CLIENTMARKER;
+		vxid.vxid |= VSL_CLIENTMARKER;
 	else if (side == VENUM(b))
-		vxid |= VSL_BACKENDMARKER;
+		vxid.vxid |= VSL_BACKENDMARKER;
 	else
 		WRONG("side");
 


More information about the varnish-commit mailing list