Mailing List Archive

commit 7d06b8b98350de557568d712ff089d3c961d0618
Author: Nils Goroll <nils.goroll@uplex.de>
Date: Wed Feb 28 11:31:16 2024 +0100

Call VRT_priv_{task,top} for each PRIV_{TASK,TOP} argument

to avoid using stale pointers after a rollback.

Before this change, we would call VRT_priv_* only once per subroutine,
which can be *) a nice performance optimization, but leaves us with stale
pointers after a rollback.

Rather than adding complications for the rollback case just to keep the
option of the "per subroutine pointer cache", just retrieve a fresh
priv pointer every time.

The other use of the per subroutine initialization was error handling,
which needs additional code outside the function arguments, simply
because a return statement is not possible within function arguments. We
removed the requirement for error handling in the previous commit by
making sure that VRT_priv_{task,top} always return a valid pointer.

Fixes https://github.com/varnish/varnish-modules/issues/222

Alternative implementation to #4060

*) It is not an optimization in all cases, for example the priv pointers
were intialized unconditionally, even if code using them was not
reached - but then again, this is something C compilers might
optimize...

diff --git a/lib/libvcc/vcc_expr.c b/lib/libvcc/vcc_expr.c
index 1e94dbf76..8cda6f12d 100644
--- a/lib/libvcc/vcc_expr.c
+++ b/lib/libvcc/vcc_expr.c
@@ -401,7 +401,6 @@ vcc_priv_arg(struct vcc *tl, const char *p, struct symbol *sym)
char buf[64];
struct inifin *ifp;
const char *f = NULL;
- struct procprivhead *marklist = NULL;

AN(sym);
AN(sym->vmod_name);
@@ -417,31 +416,18 @@ vcc_priv_arg(struct vcc *tl, const char *p, struct symbol *sym)
return (vcc_mk_expr(VOID, "&%s", buf));
}

- if (!strcmp(p, "PRIV_TASK")) {
+ if (!strcmp(p, "PRIV_TASK"))
f = "task";
- marklist = &tl->curproc->priv_tasks;
- } else if (!strcmp(p, "PRIV_TOP")) {
+ else if (!strcmp(p, "PRIV_TOP")) {
f = "top";
sym->r_methods &= VCL_MET_TASK_C;
- marklist = &tl->curproc->priv_tops;
} else {
WRONG("Wrong PRIV_ type");
}
AN(f);
- AN(marklist);
- bprintf(buf, "ARG_priv_%s_%s", f, sym->vmod_name);
-
- if (vcc_MarkPriv(tl, marklist, sym->vmod_name) == NULL)
- VSB_printf(tl->curproc->prologue,
- " struct vmod_priv *%s = "
- "VRT_priv_%s(ctx, &VGC_vmod_%s);\n"
- " if (%s == NULL) {\n"
- " VRT_fail(ctx, \"failed to get %s priv "
- "for vmod %s\");\n"
- " return;\n"
- " }\n",
- buf, f, sym->vmod_name, buf, f, sym->vmod_name);
- return (vcc_mk_expr(VOID, "%s", buf));
+
+ return (vcc_mk_expr(VOID, "VRT_priv_%s(ctx, &VGC_vmod_%s)",
+ f, sym->vmod_name));
}

struct func_arg {
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit