[master] 681c11998 Methods for vmod_priv
Nils Goroll
nils.goroll at uplex.de
Mon Nov 30 10:34:07 UTC 2020
commit 681c11998ec56dc4460a30585210f95a7850ea3b
Author: Nils Goroll <nils.goroll at uplex.de>
Date: Wed Nov 18 18:55:03 2020 +0100
Methods for vmod_priv
This is the refactoring we agreed on to enable an alternative
implementation of #3454. This PR does not yet introduce the copy
callback needed to add the functionality suggested in #3454.
We replace the .free pointer of struct vmod_priv with a pointer to a
methods struct with callbacks. As of this commit, it only contains
the .free callback renamed to .fini. The purpose of the refactoring is
to allow addition of more callbacks later.
The new struct vmod_priv_methods also contains a .type member pointing
to a string to contain an arbitrary description of the type of data any
priv holds which uses these methods.
Implementation:
relevant changes are in cache_vrt_priv.c and vrt.h, other changes are to
the documentation and bundled vmods.
The implementation is a simple refactoring for indirection of the call
to the .fini callback via the methods structure.
diff --git a/bin/varnishd/cache/cache_vrt_priv.c b/bin/varnishd/cache/cache_vrt_priv.c
index 44692af56..bb4d41ebd 100644
--- a/bin/varnishd/cache/cache_vrt_priv.c
+++ b/bin/varnishd/cache/cache_vrt_priv.c
@@ -61,6 +61,8 @@ void
pan_privs(struct vsb *vsb, const struct vrt_privs *privs)
{
struct vrt_priv *vp;
+ const struct vmod_priv *p;
+ const struct vmod_priv_methods *m;
VSB_printf(vsb, "privs = %p {\n", privs);
if (PAN_already(vsb, privs))
@@ -69,11 +71,18 @@ pan_privs(struct vsb *vsb, const struct vrt_privs *privs)
if (privs != NULL) {
VRBT_FOREACH(vp, vrt_privs, privs) {
PAN_CheckMagic(vsb, vp, VRT_PRIV_MAGIC);
+ p = vp->priv;
+ 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 f %p} vmod %jx\n",
- vp->priv->priv,
- vp->priv->len,
- vp->priv->free,
+ "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
);
}
@@ -201,9 +210,17 @@ VRT_priv_top(VRT_CTX, const void *vmod_id)
void
VRT_priv_fini(const struct vmod_priv *p)
{
+ const struct vmod_priv_methods *m;
- if (p->priv != NULL && p->free != NULL)
- p->free(p->priv);
+ m = p->methods;
+ if (m == NULL)
+ return;
+
+ CHECK_OBJ(m, VMOD_PRIV_METHODS_MAGIC);
+ if (p->priv == NULL || m->fini == NULL)
+ return;
+
+ m->fini(p->priv);
}
/*--------------------------------------------------------------------*/
diff --git a/doc/sphinx/reference/vmod.rst b/doc/sphinx/reference/vmod.rst
index 4497fe095..afc97d820 100644
--- a/doc/sphinx/reference/vmod.rst
+++ b/doc/sphinx/reference/vmod.rst
@@ -502,28 +502,48 @@ specified.
This structure contains three members::
- typedef void vmod_priv_free_f(void *);
struct vmod_priv {
- void *priv;
- int len;
- vmod_priv_free_f *free;
+ void *priv;
+ long len;
+ const struct vmod_priv_methods *methods;
};
-The "priv" and "len" elements can be used for whatever the vmod
-code wants to use them for, and the "free" element provides a
-callback to clean them up.
+The ``.priv`` and ``.len`` elements can be used for whatever the vmod
+code wants to use them for.
-If both the "priv" and "free" pointers are non-NULL when the scope
-ends, the "free" function will be called with the "priv" pointer
-as its only argument.
+``.methods`` can be an optional pointer to a struct of callbacks::
+
+ typedef void vmod_priv_fini_f(void *);
+
+ struct vmod_priv_methods {
+ unsigned magic;
+ const char *type;
+ vmod_priv_fini_f *fini;
+ };
+
+``.magic`` has to be initialized to
+``VMOD_PRIV_METHODS_MAGIC``. ``.type`` should be a descriptive name to
+help debugging.
+
+``.fini`` will be called for a non-NULL ``.priv`` of the ``struct
+vmod_priv`` when the scope ends with that ``.priv`` pointer as its
+only argument.
In the common case where a private data structure is allocated with
malloc(3) would look like this::
+ static const struct vmod_priv_methods mymethods[1] = {{
+ .magic = VMOD_PRIV_METHODS_MAGIC,
+ .type = "mystate",
+ .fini = free /* free(3) */
+ }};
+
+ // ....
+
if (priv->priv == NULL) {
priv->priv = calloc(1, sizeof(struct myfoo));
AN(priv->priv);
- priv->free = free; /* free(3) */
+ priv->methods = mymethods;
mystate = priv->priv;
mystate->foo = 21;
...
diff --git a/include/vrt.h b/include/vrt.h
index 4dea8c50a..7508a5b22 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -55,6 +55,8 @@
* 13.0 (2021-03-15)
* Calling convention for VDP implementation changed
* Added VRT_ValidHdr()
+ * struct vmod_priv_methods added
+ * struct vmod_priv free member replaced with methods
* 12.0 (2020-09-15)
* Added VRT_DirectorResolve()
* Added VCL_STRING VRT_BLOB_string(VRT_CTX, VCL_BLOB)
@@ -583,11 +585,20 @@ void VRT_Format_Proxy(struct vsb *, VCL_INT, VCL_IP, VCL_IP, VCL_STRING);
typedef int vmod_event_f(VRT_CTX, struct vmod_priv *, enum vcl_event_e);
-typedef void vmod_priv_free_f(void *);
+/* vmod_priv related */
+typedef void vmod_priv_fini_f(void *);
+
+struct vmod_priv_methods {
+ unsigned magic;
+#define VMOD_PRIV_METHODS_MAGIC 0xcea5ff99
+ const char *type;
+ vmod_priv_fini_f *fini;
+};
+
struct vmod_priv {
- void *priv;
- long len;
- vmod_priv_free_f *free;
+ void *priv;
+ long len;
+ const struct vmod_priv_methods *methods;
};
void VRT_priv_fini(const struct vmod_priv *p);
diff --git a/lib/libvmod_cookie/vmod_cookie.c b/lib/libvmod_cookie/vmod_cookie.c
index 6c8b6a3a4..fdaa2f594 100644
--- a/lib/libvmod_cookie/vmod_cookie.c
+++ b/lib/libvmod_cookie/vmod_cookie.c
@@ -84,6 +84,12 @@ cobj_free(void *p)
FREE_OBJ(vcp);
}
+static const struct vmod_priv_methods cookie_cobj_priv_methods[1] = {{
+ .magic = VMOD_PRIV_METHODS_MAGIC,
+ .type = "vmod_cookie_cobj",
+ .fini = cobj_free
+}};
+
static struct vmod_cookie *
cobj_get(struct vmod_priv *priv)
{
@@ -94,7 +100,7 @@ cobj_get(struct vmod_priv *priv)
AN(vcp);
VTAILQ_INIT(&vcp->cookielist);
priv->priv = vcp;
- priv->free = cobj_free;
+ priv->methods = cookie_cobj_priv_methods;
} else
CAST_OBJ_NOTNULL(vcp, priv->priv, VMOD_COOKIE_MAGIC);
@@ -258,6 +264,12 @@ free_re(void *priv)
AZ(vre);
}
+static const struct vmod_priv_methods cookie_re_priv_methods[1] = {{
+ .magic = VMOD_PRIV_METHODS_MAGIC,
+ .type = "vmod_cookie_re",
+ .fini = free_re
+}};
+
VCL_STRING
vmod_get_re(VRT_CTX, struct vmod_priv *priv, struct vmod_priv *priv_call,
VCL_STRING expression)
@@ -281,7 +293,7 @@ vmod_get_re(VRT_CTX, struct vmod_priv *priv, struct vmod_priv *priv_call,
}
priv_call->priv = vre;
- priv_call->free = free_re;
+ priv_call->methods = cookie_re_priv_methods;
AZ(pthread_mutex_unlock(&mtx));
}
@@ -431,7 +443,7 @@ re_filter(VRT_CTX, struct vmod_priv *priv, struct vmod_priv *priv_call,
}
priv_call->priv = vre;
- priv_call->free = free_re;
+ priv_call->methods = cookie_re_priv_methods;
AZ(pthread_mutex_unlock(&mtx));
}
diff --git a/lib/libvmod_debug/vmod_debug.c b/lib/libvmod_debug/vmod_debug.c
index a3cfa0dae..7c50561f5 100644
--- a/lib/libvmod_debug/vmod_debug.c
+++ b/lib/libvmod_debug/vmod_debug.c
@@ -225,6 +225,12 @@ xyzzy_author(VRT_CTX, VCL_ENUM person, VCL_ENUM someone)
WRONG("Illegal VMOD enum");
}
+static const struct vmod_priv_methods xyzzy_test_priv_call_methods[1] = {{
+ .magic = VMOD_PRIV_METHODS_MAGIC,
+ .type = "debug_test_priv_call",
+ .fini = free
+}};
+
VCL_VOID v_matchproto_(td_debug_test_priv_call)
xyzzy_test_priv_call(VRT_CTX, struct vmod_priv *priv)
{
@@ -232,7 +238,7 @@ xyzzy_test_priv_call(VRT_CTX, struct vmod_priv *priv)
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
if (priv->priv == NULL) {
priv->priv = strdup("BAR");
- priv->free = free;
+ priv->methods = xyzzy_test_priv_call_methods;
} else {
assert(!strcmp(priv->priv, "BAR"));
}
@@ -246,6 +252,12 @@ priv_task_free(void *ptr)
free(ptr);
}
+static const struct vmod_priv_methods xyzzy_test_priv_task_methods[1] = {{
+ .magic = VMOD_PRIV_METHODS_MAGIC,
+ .type = "debug_test_priv_task",
+ .fini = priv_task_free
+}};
+
VCL_STRING v_matchproto_(td_debug_test_priv_task)
xyzzy_test_priv_task(VRT_CTX, struct vmod_priv *priv, VCL_STRING s)
{
@@ -256,7 +268,7 @@ xyzzy_test_priv_task(VRT_CTX, struct vmod_priv *priv, VCL_STRING s)
priv, priv->priv);
} else if (priv->priv == NULL) {
priv->priv = strdup(s);
- priv->free = priv_task_free;
+ priv->methods = xyzzy_test_priv_task_methods;
VSL(SLT_Debug, 0, "test_priv_task(%p) = %p (new)",
priv, priv->priv);
} else {
@@ -271,10 +283,16 @@ xyzzy_test_priv_task(VRT_CTX, struct vmod_priv *priv, VCL_STRING s)
priv, priv->priv);
}
if (priv->priv != NULL)
- assert(priv->free == priv_task_free);
+ assert(priv->methods == xyzzy_test_priv_task_methods);
return (priv->priv);
}
+static const struct vmod_priv_methods xyzzy_test_priv_top_methods[1] = {{
+ .magic = VMOD_PRIV_METHODS_MAGIC,
+ .type = "debug_test_priv_top",
+ .fini = free
+}};
+
VCL_STRING v_matchproto_(td_debug_test_priv_top)
xyzzy_test_priv_top(VRT_CTX, struct vmod_priv *priv, VCL_STRING s)
{
@@ -282,7 +300,7 @@ xyzzy_test_priv_top(VRT_CTX, struct vmod_priv *priv, VCL_STRING s)
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
if (priv->priv == NULL) {
priv->priv = strdup(s);
- priv->free = free;
+ priv->methods = xyzzy_test_priv_top_methods;
}
return (priv->priv);
}
@@ -376,7 +394,7 @@ xyzzy_fail2(VRT_CTX)
return (1);
}
-static void v_matchproto_(vmod_priv_free_f)
+static void v_matchproto_(vmod_priv_fini_f)
priv_vcl_free(void *priv)
{
struct priv_vcl *priv_vcl;
@@ -394,6 +412,12 @@ priv_vcl_free(void *priv)
AZ(priv_vcl);
}
+static const struct vmod_priv_methods priv_vcl_methods[1] = {{
+ .magic = VMOD_PRIV_METHODS_MAGIC,
+ .type = "debug_priv_vcl_free",
+ .fini = priv_vcl_free
+}};
+
static int
event_load(VRT_CTX, struct vmod_priv *priv)
{
@@ -413,7 +437,7 @@ event_load(VRT_CTX, struct vmod_priv *priv)
priv_vcl->foo = strdup("FOO");
AN(priv_vcl->foo);
priv->priv = priv_vcl;
- priv->free = priv_vcl_free;
+ priv->methods = priv_vcl_methods;
VRT_AddVFP(ctx, &xyzzy_rot13);
@@ -977,7 +1001,7 @@ xyzzy_store_ip(VRT_CTX, VCL_IP ip)
return;
}
- AZ(priv->free);
+ AZ(priv->methods);
assert(VSA_Sane(ip));
priv->priv = TRUST_ME(ip);
}
@@ -992,7 +1016,7 @@ xyzzy_get_ip(VRT_CTX)
priv = VRT_priv_task(ctx, &store_ip_token);
AN(priv);
- AZ(priv->free);
+ AZ(priv->methods);
ip = priv->priv;
assert(VSA_Sane(ip));
@@ -1146,6 +1170,12 @@ fail_f(void *priv)
VRT_fail(ctx, "thou shalt not rollet back");
}
+static const struct vmod_priv_methods xyzzy_fail_rollback_methods[1] = {{
+ .magic = VMOD_PRIV_METHODS_MAGIC,
+ .type = "debug_fail_rollback",
+ .fini = fail_f
+}};
+
VCL_VOID v_matchproto_(td_xyzzy_debug_fail_rollback)
xyzzy_fail_rollback(VRT_CTX)
{
@@ -1161,12 +1191,12 @@ xyzzy_fail_rollback(VRT_CTX)
if (p->priv != NULL) {
assert(p->priv == ctx);
- assert(p->free == fail_f);
+ assert(p->methods == xyzzy_fail_rollback_methods);
return;
}
p->priv = TRUST_ME(ctx);
- p->free = fail_f;
+ p->methods = xyzzy_fail_rollback_methods;
}
VCL_VOID v_matchproto_(td_xyzzy_debug_ok_rollback)
@@ -1183,7 +1213,7 @@ xyzzy_ok_rollback(VRT_CTX)
}
p->priv = NULL;
- p->free = NULL;
+ p->methods = NULL;
}
VCL_STRING v_matchproto_(td_xyzzy_debug_re_quote)
diff --git a/lib/libvmod_directors/shard_cfg.c b/lib/libvmod_directors/shard_cfg.c
index 312e9c722..1a0cb40e4 100644
--- a/lib/libvmod_directors/shard_cfg.c
+++ b/lib/libvmod_directors/shard_cfg.c
@@ -86,7 +86,7 @@ change_reconfigure(struct shard_change *change, VCL_INT replicas);
* a PRIV_TASK state, which we work in reconfigure.
*/
-static void v_matchproto_(vmod_priv_free_f)
+static void v_matchproto_(vmod_priv_fini_f)
shard_change_fini(void * priv)
{
struct shard_change *change;
@@ -99,6 +99,12 @@ shard_change_fini(void * priv)
(void) change_reconfigure(change, 67);
}
+static const struct vmod_priv_methods shard_change_priv_methods[1] = {{
+ .magic = VMOD_PRIV_METHODS_MAGIC,
+ .type = "vmod_directors_shard_cfg",
+ .fini = shard_change_fini
+}};
+
static struct shard_change *
shard_change_get(VRT_CTX, struct sharddir * const shardd)
{
@@ -132,7 +138,7 @@ shard_change_get(VRT_CTX, struct sharddir * const shardd)
change->shardd = shardd;
VSTAILQ_INIT(&change->tasks);
task->priv = change;
- task->free = shard_change_fini;
+ task->methods = shard_change_priv_methods;
return (change);
}
diff --git a/lib/libvmod_std/vmod_std_fileread.c b/lib/libvmod_std/vmod_std_fileread.c
index df7a160d1..1da8b670b 100644
--- a/lib/libvmod_std/vmod_std_fileread.c
+++ b/lib/libvmod_std/vmod_std_fileread.c
@@ -83,6 +83,12 @@ free_frfile(void *ptr)
}
}
+static const struct vmod_priv_methods frfile_methods[1] = {{
+ .magic = VMOD_PRIV_METHODS_MAGIC,
+ .type = "vmod_std_fileread",
+ .fini = free_frfile
+}};
+
static struct frfile *
find_frfile(struct vmod_priv *priv, VCL_STRING file_name)
{
@@ -112,7 +118,7 @@ find_frfile(struct vmod_priv *priv, VCL_STRING file_name)
}
AZ(pthread_mutex_unlock(&frmtx));
if (frf != NULL) {
- priv->free = free_frfile;
+ priv->methods = frfile_methods;
priv->priv = frf;
return (frf);
}
@@ -127,7 +133,7 @@ find_frfile(struct vmod_priv *priv, VCL_STRING file_name)
frf->contents = s;
frf->blob->blob = s;
frf->blob->len = (size_t)sz;
- priv->free = free_frfile;
+ priv->methods = frfile_methods;
priv->priv = frf;
AZ(pthread_mutex_lock(&frmtx));
VTAILQ_INSERT_HEAD(&frlist, frf, list);
More information about the varnish-commit
mailing list