Hi,

Please find attached a patch to amend this commit and replace the void
pointer with a union type instead.

If you don't like the idea of the union type, there's a typo you can fix with:

   sed -i s/SHA256ctx/SHA256Context/ include/vrt.h

Cheers,
Dridi

On Thu, May 14, 2015 at 11:43 AM, Poul-Henning Kamp <[email protected]> wrote:
>
> commit d9c19168e544747d0f8d08499a484a883231318c
> Author: Poul-Henning Kamp <[email protected]>
> Date:   Thu May 14 09:42:46 2015 +0000
>
>     Add a per-method specific pointer to the vrt-context, and use
>     it to slim down req/busyobj a tiny bit.
From b7ab2aa07efd891dff0267cf97d64c76714bccb2 Mon Sep 17 00:00:00 2001
From: Dridi Boukelmoune <[email protected]>
Date: Thu, 14 May 2015 16:08:03 +0200
Subject: [PATCH] Use a union instead of *void for VRT specific args

It allows slightly stronger typing at compile-time and make `NULL` args
more explicit by using the `no_specific` constant.
---
 bin/varnishd/cache/cache.h         |  4 +++-
 bin/varnishd/cache/cache_fetch.c   | 23 ++++++++++++-----------
 bin/varnishd/cache/cache_req_fsm.c | 35 +++++++++++++++++++----------------
 bin/varnishd/cache/cache_vcl.c     |  4 ++--
 bin/varnishd/cache/cache_vrt.c     | 10 +++++-----
 include/vrt.h                      | 17 +++++++++++------
 6 files changed, 52 insertions(+), 41 deletions(-)

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 7986cb3..0f7822d 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -1076,9 +1076,11 @@ void VCL_Poll(void);
 const char *VCL_Return_Name(unsigned);
 const char *VCL_Method_Name(unsigned);
 
+union vrt_specific;
+
 #define VCL_MET_MAC(l,u,b) \
     void VCL_##l##_method(struct VCL_conf *, struct worker *, struct req *, \
-	struct busyobj *bo, void *specific);
+	struct busyobj *bo, union vrt_specific specific);
 #include "tbl/vcl_returns.h"
 #undef VCL_MET_MAC
 
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index b29eb25..d0e9cec 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -34,6 +34,7 @@
 #include "cache_filter.h"
 #include "hash/hash_slinger.h"
 #include "vcl.h"
+#include "vrt.h"
 #include "vtim.h"
 
 /*--------------------------------------------------------------------
@@ -274,7 +275,7 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 
 	http_PrintfHeader(bo->bereq, "X-Varnish: %u", VXID(bo->vsl->wid));
 
-	VCL_backend_fetch_method(bo->vcl, wrk, NULL, bo, NULL);
+	VCL_backend_fetch_method(bo->vcl, wrk, NULL, bo, no_specific);
 
 	bo->uncacheable = bo->do_pass;
 	if (wrk->handling == VCL_RET_ABANDON)
@@ -426,7 +427,7 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 	bo->vfc->http = bo->beresp;
 	bo->vfc->esi_req = bo->bereq;
 
-	VCL_backend_response_method(bo->vcl, wrk, NULL, bo, NULL);
+	VCL_backend_response_method(bo->vcl, wrk, NULL, bo, no_specific);
 
 	if (wrk->handling == VCL_RET_ABANDON) {
 		bo->doclose = SC_RESP_CLOSE;
@@ -770,7 +771,7 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
 	ssize_t l, ll, o;
 	double now;
 	uint8_t *ptr;
-	struct vsb *synth_body;
+	union vrt_specific synth_body;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
@@ -781,8 +782,8 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
 
 	AN(bo->fetch_objcore->flags & OC_F_BUSY);
 
-	synth_body = VSB_new_auto();
-	AN(synth_body);
+	synth_body.vsb = VSB_new_auto();
+	AN(synth_body.vsb);
 
 	// XXX: reset all beresp flags ?
 
@@ -798,10 +799,10 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
 
 	VCL_backend_error_method(bo->vcl, wrk, NULL, bo, synth_body);
 
-	AZ(VSB_finish(synth_body));
+	AZ(VSB_finish(synth_body.vsb));
 
 	if (wrk->handling == VCL_RET_RETRY) {
-		VSB_delete(synth_body);
+		VSB_delete(synth_body.vsb);
 
 		bo->doclose = SC_RESP_CLOSE;
 		if (bo->director_state != DIR_S_NULL)
@@ -822,22 +823,22 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
 	bo->vfc->esi_req = bo->bereq;
 
 	if (vbf_beresp2obj(bo)) {
-		VSB_delete(synth_body);
+		VSB_delete(synth_body.vsb);
 		return (F_STP_FAIL);
 	}
 
-	ll = VSB_len(synth_body);
+	ll = VSB_len(synth_body.vsb);
 	o = 0;
 	while (ll > 0) {
 		l = ll;
 		if (VFP_GetStorage(bo->vfc, &l, &ptr) != VFP_OK)
 			break;
-		memcpy(ptr, VSB_data(synth_body) + o, l);
+		memcpy(ptr, VSB_data(synth_body.vsb) + o, l);
 		VBO_extend(bo, l);
 		ll -= l;
 		o += l;
 	}
-	VSB_delete(synth_body);
+	VSB_delete(synth_body.vsb);
 
 	HSH_Unbusy(wrk, bo->fetch_objcore);
 	VBO_setstate(bo, BOS_FINISHED);
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index c4594a7..2f27a79 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -45,6 +45,7 @@
 #include "hash/hash_slinger.h"
 #include "vcl.h"
 #include "vsha256.h"
+#include "vrt.h"
 #include "vtim.h"
 
 static void
@@ -133,7 +134,7 @@ cnt_deliver(struct worker *wrk, struct req *req)
 	    !RFC2616_Req_Gzip(req->http))
 		RFC2616_Weaken_Etag(req->resp);
 
-	VCL_deliver_method(req->vcl, wrk, req, NULL, NULL);
+	VCL_deliver_method(req->vcl, wrk, req, NULL, no_specific);
 	VSLb_ts_req(req, "Process", W_TIM_real(wrk));
 
 	/* Stop the insanity before it turns "Hotel California" on us */
@@ -211,7 +212,7 @@ cnt_synth(struct worker *wrk, struct req *req)
 {
 	struct http *h;
 	double now;
-	struct vsb *synth_body;
+	union vrt_specific synth_body;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
@@ -232,21 +233,21 @@ cnt_synth(struct worker *wrk, struct req *req)
 	http_PrintfHeader(req->resp, "X-Varnish: %u", VXID(req->vsl->wid));
 	http_PutResponse(h, "HTTP/1.1", req->err_code, req->err_reason);
 
-	synth_body = VSB_new_auto();
-	AN(synth_body);
+	synth_body.vsb = VSB_new_auto();
+	AN(synth_body.vsb);
 
 	VCL_synth_method(req->vcl, wrk, req, NULL, synth_body);
 
 	http_Unset(h, H_Content_Length);
 
-	AZ(VSB_finish(synth_body));
+	AZ(VSB_finish(synth_body.vsb));
 
 	/* Discard any lingering request body before delivery */
 	(void)VRB_Ignore(req);
 
 	if (wrk->handling == VCL_RET_RESTART) {
 		HTTP_Setup(h, req->ws, req->vsl, SLT_RespMethod);
-		VSB_delete(synth_body);
+		VSB_delete(synth_body.vsb);
 		req->req_step = R_STP_RESTART;
 		return (REQ_FSM_MORE);
 	}
@@ -261,20 +262,20 @@ cnt_synth(struct worker *wrk, struct req *req)
 		ssize_t sz, szl;
 		uint8_t *ptr;
 
-		szl = VSB_len(synth_body);
+		szl = VSB_len(synth_body.vsb);
 		assert(szl >= 0);
 		if (szl > 0) {
 			sz = szl;
 			AN(ObjGetSpace(wrk, req->objcore, &sz, &ptr));
 			assert(sz >= szl);
-			memcpy(ptr, VSB_data(synth_body), szl);
+			memcpy(ptr, VSB_data(synth_body.vsb), szl);
 			ObjExtend(wrk, req->objcore, szl);
 		}
 
 		cnt_vdp(req, NULL);
 		(void)HSH_DerefObjCore(wrk, &req->objcore);
 	}
-	VSB_delete(synth_body);
+	VSB_delete(synth_body.vsb);
 
 	VSLb_ts_req(req, "Resp", W_TIM_real(wrk));
 
@@ -386,7 +387,7 @@ cnt_lookup(struct worker *wrk, struct req *req)
 
 	VSLb(req->vsl, SLT_Hit, "%u", ObjGetXID(wrk, req->objcore));
 
-	VCL_hit_method(req->vcl, wrk, req, NULL, NULL);
+	VCL_hit_method(req->vcl, wrk, req, NULL, no_specific);
 
 	switch (wrk->handling) {
 	case VCL_RET_DELIVER:
@@ -461,7 +462,7 @@ cnt_miss(struct worker *wrk, struct req *req)
 	CHECK_OBJ_NOTNULL(req->vcl, VCL_CONF_MAGIC);
 	CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC);
 
-	VCL_miss_method(req->vcl, wrk, req, NULL, NULL);
+	VCL_miss_method(req->vcl, wrk, req, NULL, no_specific);
 	switch (wrk->handling) {
 	case VCL_RET_FETCH:
 		wrk->stats->cache_miss++;
@@ -502,7 +503,7 @@ cnt_pass(struct worker *wrk, struct req *req)
 	CHECK_OBJ_NOTNULL(req->vcl, VCL_CONF_MAGIC);
 	AZ(req->objcore);
 
-	VCL_pass_method(req->vcl, wrk, req, NULL, NULL);
+	VCL_pass_method(req->vcl, wrk, req, NULL, no_specific);
 	switch (wrk->handling) {
 	case VCL_RET_SYNTH:
 		req->req_step = R_STP_SYNTH;
@@ -548,7 +549,7 @@ cnt_pipe(struct worker *wrk, struct req *req)
 	http_PrintfHeader(bo->bereq, "X-Varnish: %u", VXID(req->vsl->wid));
 	http_SetHeader(bo->bereq, "Connection: close");
 
-	VCL_pipe_method(req->vcl, wrk, req, bo, NULL);
+	VCL_pipe_method(req->vcl, wrk, req, bo, no_specific);
 
 	if (wrk->handling == VCL_RET_SYNTH)
 		INCOMPL();
@@ -602,6 +603,7 @@ cnt_recv(struct worker *wrk, struct req *req)
 {
 	unsigned recv_handling;
 	struct SHA256Context sha256ctx;
+	union vrt_specific hash_specific;
 	const char *xff;
 	char *ci, *cp;
 
@@ -649,7 +651,7 @@ cnt_recv(struct worker *wrk, struct req *req)
 
 	http_CollectHdr(req->http, H_Cache_Control);
 
-	VCL_recv_method(req->vcl, wrk, req, NULL, NULL);
+	VCL_recv_method(req->vcl, wrk, req, NULL, no_specific);
 
 	/* Attempts to cache req.body may fail */
 	if (req->req_body_status == REQ_BODY_FAIL) {
@@ -671,7 +673,8 @@ cnt_recv(struct worker *wrk, struct req *req)
 	}
 
 	SHA256_Init(&sha256ctx);
-	VCL_hash_method(req->vcl, wrk, req, NULL, &sha256ctx);
+	hash_specific.sha = &sha256ctx;
+	VCL_hash_method(req->vcl, wrk, req, NULL, hash_specific);
 	assert(wrk->handling == VCL_RET_LOOKUP);
 	SHA256_Final(req->digest, &sha256ctx);
 
@@ -739,7 +742,7 @@ cnt_purge(struct worker *wrk, struct req *req)
 
 	AZ(HSH_DerefObjCore(wrk, &boc));
 
-	VCL_purge_method(req->vcl, wrk, req, NULL, NULL);
+	VCL_purge_method(req->vcl, wrk, req, NULL, no_specific);
 	switch (wrk->handling) {
 	case VCL_RET_RESTART:
 		req->req_step = R_STP_RESTART;
diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index a453cf3..287a39a 100644
--- a/bin/varnishd/cache/cache_vcl.c
+++ b/bin/varnishd/cache/cache_vcl.c
@@ -461,7 +461,7 @@ ccf_config_show(struct cli *cli, const char * const *av, void *priv)
 
 static void
 vcl_call_method(struct worker *wrk, struct req *req, struct busyobj *bo,
-    void *specific, unsigned method, vcl_func_f *func)
+    union vrt_specific specific, unsigned method, vcl_func_f *func)
 {
 	char *aws;
 	struct vsl_log *vsl = NULL;
@@ -515,7 +515,7 @@ vcl_call_method(struct worker *wrk, struct req *req, struct busyobj *bo,
 #define VCL_MET_MAC(func, upper, bitmap)				\
 void									\
 VCL_##func##_method(struct VCL_conf *vcl, struct worker *wrk,		\
-     struct req *req, struct busyobj *bo, void *specific)		\
+     struct req *req, struct busyobj *bo, union vrt_specific specific)	\
 {									\
 									\
 	CHECK_OBJ_NOTNULL(vcl, VCL_CONF_MAGIC);				\
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 5a12c63..ac5a466 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -264,21 +264,21 @@ VRT_hashdata(VRT_CTX, const char *str, ...)
 
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 	CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
-	AN(ctx->specific);
-	HSH_AddString(ctx->req, ctx->specific, str);
+	AN(ctx->specific.sha);
+	HSH_AddString(ctx->req, ctx->specific.sha, str);
 	va_start(ap, str);
 	while (1) {
 		p = va_arg(ap, const char *);
 		if (p == vrt_magic_string_end)
 			break;
-		HSH_AddString(ctx->req, ctx->specific, p);
+		HSH_AddString(ctx->req, ctx->specific.sha, p);
 	}
 	va_end(ap);
 	/*
 	 * Add a 'field-separator' to make it more difficult to
 	 * manipulate the hash.
 	 */
-	HSH_AddString(ctx->req, ctx->specific, NULL);
+	HSH_AddString(ctx->req, ctx->specific.sha, NULL);
 }
 
 /*--------------------------------------------------------------------*/
@@ -387,7 +387,7 @@ VRT_synth_page(VRT_CTX, const char *str, ...)
 	const char *p;
 	struct vsb *vsb;
 
-	CAST_OBJ_NOTNULL(vsb, ctx->specific, VSB_MAGIC);
+	CAST_OBJ_NOTNULL(vsb, ctx->specific.vsb, VSB_MAGIC);
 	va_start(ap, str);
 	p = str;
 	while (p != vrt_magic_string_end) {
diff --git a/include/vrt.h b/include/vrt.h
index 97c4e92..bac94c1 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -31,6 +31,8 @@
  * NB: When this file is changed, lib/libvcc/generate.py *MUST* be rerun.
  */
 
+#include <stdlib.h>
+
 /***********************************************************************
  * Major and minor VRT API versions.
  *
@@ -86,6 +88,14 @@ typedef void				VCL_VOID;
  * functions.
  */
 
+union vrt_specific {
+	void			*none;
+	struct SHA256Context	*sha;
+	struct vsb		*vsb;
+};
+
+static const union vrt_specific no_specific = { .none = NULL };
+
 struct vrt_ctx {
 	unsigned			magic;
 #define VRT_CTX_MAGIC			0x6bb8f0db
@@ -108,12 +118,7 @@ struct vrt_ctx {
 
 	double				now;
 
-	/*
-	 * method specific argument:
-	 *    hash:		struct SHA256ctx
-	 *    synth+error:	struct vsb *
-	 */
-	void				*specific;
+	union vrt_specific		specific;
 };
 
 #define VRT_CTX		const struct vrt_ctx *ctx
-- 
2.1.0

_______________________________________________
varnish-dev mailing list
[email protected]
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Reply via email to