[master] 545055090 Change type of VCL_BLOB to newly introduced struct vrt_blob
Nils Goroll
nils.goroll at uplex.de
Wed Jan 23 18:14:02 UTC 2019
commit 5450550906d0809dfc21ae9d3a5b14446c1809d4
Author: Nils Goroll <nils.goroll at uplex.de>
Date: Wed Jan 23 18:58:30 2019 +0100
Change type of VCL_BLOB to newly introduced struct vrt_blob
Re-using struct vmod_priv helped simplicity, but had the following
shortcomings:
- in ddfda3d7c996e67a1894fc3955eb546ebd0b721b we documented what was
long implied: return values of vmod functions are assumed immutable
and so are blobs.
So the blob pointer should be const, yet the struct vmod_priv pointer
can't be.
- the struct vmod_priv free pointer implies that there would be
automatic memory management provided by varnish core, yet this does not
exist for a good reason: We would otherwise need to track all blobs
ever returned by vmod functions/methods.
So we turn the data pointer into a const and remove the free function
callback.
We also add a type field, which is to be viewed similar to the miniobj
magic, except that it should not be asserted upon. The type field is
intended for additional (yet unreliable) checks of vmods using BLOBs
to carry around vmod-specific private data.
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index e1ac810c4..45b4e1399 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -839,21 +839,24 @@ VRT_ipcmp(VCL_IP sua1, VCL_IP sua2)
return (VSA_Compare_IP(sua1, sua2));
}
+/*
+ * the pointer passed as src must have at least VCL_TASK lifetime
+ */
VCL_BLOB
-VRT_blob(VRT_CTX, const char *err, const void *src, size_t len)
+VRT_blob(VRT_CTX, const char *err, const void *src, size_t len, unsigned type)
{
- struct vmod_priv *p;
- void *d;
+ struct vrt_blob *p;
p = (void *)WS_Alloc(ctx->ws, sizeof *p);
- d = WS_Copy(ctx->ws, src, len);
- if (p == NULL || d == NULL) {
+ if (p == NULL) {
VRT_fail(ctx, "Workspace overflow (%s)", err);
return (NULL);
}
- memset(p, 0, sizeof *p);
+
+ p->type = type;
p->len = len;
- p->priv = d;
+ p->blob = src;
+
return (p);
}
diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c
index a141df54d..9b4f5fedb 100644
--- a/bin/varnishd/cache/cache_vrt_var.c
+++ b/bin/varnishd/cache/cache_vrt_var.c
@@ -891,12 +891,16 @@ VRT_BODY_L(resp)
/*--------------------------------------------------------------------*/
+ /* digest */
+#define BLOB_HASH_TYPE 0x00d16357
+
VCL_BLOB
VRT_r_req_hash(VRT_CTX)
{
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
CHECK_OBJ_NOTNULL(ctx->req, REQ_MAGIC);
- return (VRT_blob(ctx, "req.hash", ctx->req->digest, DIGEST_LEN));
+ return (VRT_blob(ctx, "req.hash", ctx->req->digest, DIGEST_LEN,
+ BLOB_HASH_TYPE));
}
VCL_BLOB
@@ -904,7 +908,8 @@ VRT_r_bereq_hash(VRT_CTX)
{
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
- return (VRT_blob(ctx, "bereq.hash", ctx->bo->digest, DIGEST_LEN));
+ return (VRT_blob(ctx, "bereq.hash", ctx->bo->digest, DIGEST_LEN,
+ BLOB_HASH_TYPE));
}
/*--------------------------------------------------------------------*/
diff --git a/include/vrt.h b/include/vrt.h
index 35d9dcaa1..5ad307294 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -56,6 +56,8 @@
* HTTP_Copy() removed
* HTTP_Dup() added
* HTTP_Clone() added
+ * changed type of VCL_BLOB to newly introduced struct vrt_blob *
+ * changed VRT_blob()
* 8.0 (2018-09-15)
* VRT_Strands() added
* VRT_StrandsWS() added
@@ -118,7 +120,7 @@
* vrt_acl type added
*/
-#define VRT_MAJOR_VERSION 8U
+#define VRT_MAJOR_VERSION 9U
#define VRT_MINOR_VERSION 0U
@@ -150,6 +152,32 @@ struct strands {
const char **p;
};
+/*
+ * VCL_BLOB:
+ *
+ * opaque, immutable data (pointer + length), minimum lifetime is the VCL task.
+ *
+ * type (optional) is owned by the creator of the blob. blob consumers may use
+ * it for checks, but should not assert on it.
+ *
+ * The data behind the blob pointer is assumed to be immutable for the blob's
+ * lifetime.
+ *
+ * Memory management is either implicit or up to the vmod:
+ *
+ * - memory for shortlived blobs should come from the respective workspace
+ *
+ * - management of memory for longer lived blobs is up to the vmod
+ * (in which case the blob will probably be embedded in an object or
+ * referenced by other state with vcl lifetime)
+ */
+
+struct vrt_blob {
+ unsigned type;
+ size_t len;
+ const void *blob;
+};
+
/***********************************************************************
* This is the central definition of the mapping from VCL types to
* C-types. The python scripts read these from here.
@@ -158,7 +186,7 @@ struct strands {
typedef const struct vrt_acl * VCL_ACL;
typedef const struct director * VCL_BACKEND;
-typedef const struct vmod_priv * VCL_BLOB;
+typedef const struct vrt_blob * VCL_BLOB;
typedef const char * VCL_BODY;
typedef unsigned VCL_BOOL;
typedef int64_t VCL_BYTES;
@@ -417,7 +445,7 @@ VCL_VOID VRT_hashdata(VRT_CTX, const char *str, ...);
int VRT_strcmp(const char *s1, const char *s2);
void VRT_memmove(void *dst, const void *src, unsigned len);
VCL_BOOL VRT_ipcmp(VCL_IP, VCL_IP);
-VCL_BLOB VRT_blob(VRT_CTX, const char *, const void *, size_t);
+VCL_BLOB VRT_blob(VRT_CTX, const char *, const void *, size_t, unsigned);
VCL_VOID VRT_Rollback(VRT_CTX, VCL_HTTP);
diff --git a/lib/libvmod_blob/vmod_blob.c b/lib/libvmod_blob/vmod_blob.c
index 28e486856..66b9f4e24 100644
--- a/lib/libvmod_blob/vmod_blob.c
+++ b/lib/libvmod_blob/vmod_blob.c
@@ -34,10 +34,13 @@
#include "vcc_if.h"
#include "vmod_blob.h"
+#define VMOD_BLOB_TYPE 0xfade4faa
+
struct vmod_blob_blob {
unsigned magic;
#define VMOD_BLOB_MAGIC 0xfade4fa9
- struct vmod_priv blob;
+ struct vrt_blob blob;
+ void *freeptr;
char *encoding[__MAX_ENCODING][2];
pthread_mutex_t lock;
};
@@ -105,14 +108,12 @@ static const struct vmod_blob_fptr {
static char empty[1] = { '\0' };
-static const struct vmod_priv null_blob[1] =
-{
- {
- .priv = empty,
- .len = 0,
- .free = NULL
- }
-};
+static const struct vrt_blob null_blob[1] = {{
+#define VMOD_BLOB_NULL_TYPE 0xfade4fa0
+ .type = VMOD_BLOB_NULL_TYPE,
+ .len = 0,
+ .blob = empty,
+}};
static enum encoding
parse_encoding(VCL_ENUM e)
@@ -187,6 +188,7 @@ vmod_blob__init(VRT_CTX, struct vmod_blob_blob **blobp, const char *vcl_name,
{
struct vmod_blob_blob *b;
enum encoding dec = parse_encoding(decs);
+ void *buf;
ssize_t len;
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
@@ -201,36 +203,37 @@ vmod_blob__init(VRT_CTX, struct vmod_blob_blob **blobp, const char *vcl_name,
*blobp = b;
AZ(pthread_mutex_init(&b->lock, NULL));
+ b->blob.type = VMOD_BLOB_TYPE;
+
len = decode_l(dec, strings);
if (len == 0)
return;
assert(len > 0);
- b->blob.priv = malloc(len);
- if (b->blob.priv == NULL) {
+ buf = malloc(len);
+ if (buf == NULL) {
VERRNOMEM(ctx, "cannot create blob %s", vcl_name);
return;
}
errno = 0;
- len = func[dec].decode(dec, b->blob.priv, len, -1, strings);
+ len = func[dec].decode(dec, buf, len, -1, strings);
if (len == -1) {
assert(errno == EINVAL);
- free(b->blob.priv);
- b->blob.priv = NULL;
+ free(buf);
VERR(ctx, "cannot create blob %s, illegal encoding beginning "
"with \"%s\"", vcl_name, strings->p[0]);
return;
}
if (len == 0) {
- b->blob.len = 0;
- free(b->blob.priv);
- b->blob.priv = NULL;
+ free(buf);
+ memcpy(&b->blob, null_blob, sizeof b->blob);
return;
}
b->blob.len = len;
+ b->blob.blob = b->freeptr = buf;
}
VCL_BLOB v_matchproto_(td_blob_blob_get)
@@ -238,7 +241,7 @@ vmod_blob_get(VRT_CTX, struct vmod_blob_blob *b)
{
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
CHECK_OBJ_NOTNULL(b, VMOD_BLOB_MAGIC);
- return &b->blob;
+ return (&b->blob);
}
VCL_STRING v_matchproto_(td_blob_blob_encode)
@@ -276,7 +279,7 @@ vmod_blob_encode(VRT_CTX, struct vmod_blob_blob *b, VCL_ENUM encs,
len =
func[enc].encode(
enc, kase, s, len,
- b->blob.priv,
+ b->blob.blob,
b->blob.len);
assert(len >= 0);
if (len == 0) {
@@ -304,9 +307,9 @@ vmod_blob__fini(struct vmod_blob_blob **blobp)
b = *blobp;
*blobp = NULL;
CHECK_OBJ(b, VMOD_BLOB_MAGIC);
- if (b->blob.priv != NULL) {
- free(b->blob.priv);
- b->blob.priv = NULL;
+ if (b->freeptr != NULL) {
+ free(b->freeptr);
+ b->blob.blob = NULL;
}
for (int i = 0; i < __MAX_ENCODING; i++)
for (int j = 0; j < 2; j++) {
@@ -326,9 +329,7 @@ VCL_BLOB v_matchproto_(td_blob_decode)
vmod_decode(VRT_CTX, VCL_ENUM decs, VCL_INT length, VCL_STRANDS strings)
{
enum encoding dec = parse_encoding(decs);
- struct vmod_priv *b;
char *buf;
- uintptr_t snap;
ssize_t len;
unsigned space;
@@ -337,12 +338,6 @@ vmod_decode(VRT_CTX, VCL_ENUM decs, VCL_INT length, VCL_STRANDS strings)
AN(strings);
CHECK_OBJ_NOTNULL(ctx->ws, WS_MAGIC);
- snap = WS_Snapshot(ctx->ws);
- if ((b = WS_Alloc(ctx->ws, sizeof(struct vmod_priv))) == NULL) {
- ERRNOMEM(ctx, "cannot decode");
- return NULL;
- }
-
buf = WS_Front(ctx->ws);
space = WS_Reserve(ctx->ws, 0);
@@ -354,19 +349,17 @@ vmod_decode(VRT_CTX, VCL_ENUM decs, VCL_INT length, VCL_STRANDS strings)
if (len == -1) {
err_decode(ctx, strings->p[0]);
WS_Release(ctx->ws, 0);
- WS_Reset(ctx->ws, snap);
return NULL;
}
if (len == 0) {
WS_Release(ctx->ws, 0);
- WS_Reset(ctx->ws, snap);
return null_blob;
}
WS_Release(ctx->ws, len);
- b->priv = buf;
- b->len = len;
- b->free = NULL;
- return (b);
+
+ assert(len > 0);
+
+ return (VRT_blob(ctx, "blob.decode", buf, len, VMOD_BLOB_TYPE));
}
static VCL_STRING
@@ -387,7 +380,7 @@ encode(VRT_CTX, enum encoding enc, enum case_e kase, VCL_BLOB b)
buf = WS_Front(ctx->ws);
space = WS_Reserve(ctx->ws, 0);
- len = func[enc].encode(enc, kase, buf, space, b->priv, b->len);
+ len = func[enc].encode(enc, kase, buf, space, b->blob, b->len);
if (len == -1) {
ERRNOMEM(ctx, "cannot encode");
@@ -424,7 +417,7 @@ vmod_transcode(VRT_CTX, VCL_ENUM decs, VCL_ENUM encs, VCL_ENUM case_s,
enum encoding dec = parse_encoding(decs);
enum encoding enc = parse_encoding(encs);
enum case_e kase = parse_case(case_s);
- struct vmod_priv b;
+ struct vrt_blob b;
VCL_STRING r;
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
@@ -444,15 +437,15 @@ vmod_transcode(VRT_CTX, VCL_ENUM decs, VCL_ENUM encs, VCL_ENUM case_s,
size_t l = decode_l(dec, strings);
if (l == 0)
return "";
+
/* XXX: handle stack overflow? */
char buf[l];
- b.free = NULL;
- b.priv = buf;
if (length <= 0)
length = -1;
errno = 0;
b.len = func[dec].decode(dec, buf, l, length, strings);
+ b.blob = buf;
if (b.len == -1) {
err_decode(ctx, strings->p[0]);
@@ -487,7 +480,7 @@ vmod_same(VRT_CTX, VCL_BLOB b1, VCL_BLOB b2)
return 1;
if (b1 == NULL || b2 == NULL)
return 0;
- return (b1->len == b2->len && b1->priv == b2->priv);
+ return (b1->len == b2->len && b1->blob == b2->blob);
}
VCL_BOOL v_matchproto_(td_blob_equal)
@@ -501,11 +494,11 @@ vmod_equal(VRT_CTX, VCL_BLOB b1, VCL_BLOB b2)
return 0;
if (b1->len != b2->len)
return 0;
- if (b1->priv == b2->priv)
+ if (b1->blob == b2->blob)
return 1;
- if (b1->priv == NULL || b2->priv == NULL)
+ if (b1->blob == NULL || b2->blob == NULL)
return 0;
- return (memcmp(b1->priv, b2->priv, b1->len) == 0);
+ return (memcmp(b1->blob, b2->blob, b1->len) == 0);
}
VCL_INT v_matchproto_(td_blob_length)
@@ -521,21 +514,20 @@ vmod_length(VRT_CTX, VCL_BLOB b)
VCL_BLOB v_matchproto_(td_blob_sub)
vmod_sub(VRT_CTX, VCL_BLOB b, VCL_BYTES n, VCL_BYTES off)
{
- uintptr_t snap;
- struct vmod_priv *sub;
-
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
assert(n >= 0);
assert(off >= 0);
- if (b == NULL || b->len == 0 || b->priv == NULL) {
+ if (b == NULL || b->len == 0 || b->blob == NULL) {
ERR(ctx, "blob is empty in blob.sub()");
return NULL;
}
- assert(b->len >= 0);
+
+ assert(b->len > 0);
+
if (off + n > b->len) {
VERR(ctx, "size %jd from offset %jd requires more bytes than "
- "blob length %d in blob.sub()",
+ "blob length %zd in blob.sub()",
(intmax_t)n, (intmax_t)off, b->len);
return NULL;
}
@@ -543,17 +535,7 @@ vmod_sub(VRT_CTX, VCL_BLOB b, VCL_BYTES n, VCL_BYTES off)
if (n == 0)
return null_blob;
- snap = WS_Snapshot(ctx->ws);
- if ((sub = WS_Alloc(ctx->ws, sizeof(*sub))) == NULL) {
- ERRNOMEM(ctx, "Allocating BLOB result in blob.sub()");
- return NULL;
- }
- if ((sub->priv = WS_Alloc(ctx->ws, n)) == NULL) {
- VERRNOMEM(ctx, "Allocating %jd bytes in blob.sub()", (intmax_t)n);
- WS_Reset(ctx->ws, snap);
- return NULL;
- }
- memcpy(sub->priv, (char *)b->priv + off, n);
- sub->len = n;
- return sub;
+
+ return (VRT_blob(ctx, "blob.sub()",
+ (const char *)b->blob + off, n, b->type));
}
diff --git a/lib/libvmod_directors/vmod_shard.c b/lib/libvmod_directors/vmod_shard.c
index 3f4924bc3..9e8abcbc0 100644
--- a/lib/libvmod_directors/vmod_shard.c
+++ b/lib/libvmod_directors/vmod_shard.c
@@ -113,6 +113,8 @@ enum vmod_directors_shard_param_scope {
struct vmod_directors_shard_param;
+#define VMOD_SHARD_SHARD_PARAM_BLOB 0xdf5ca116
+
struct vmod_directors_shard_param {
unsigned magic;
#define VMOD_SHARD_SHARD_PARAM_MAGIC 0xdf5ca117
@@ -157,7 +159,7 @@ shard_param_task(VRT_CTX, const void *id,
const struct vmod_directors_shard_param *pa);
static const struct vmod_directors_shard_param *
-shard_param_blob(const VCL_BLOB blob);
+shard_param_blob(VCL_BLOB blob);
static const struct vmod_directors_shard_param *
vmod_shard_param_read(VRT_CTX, const void *id,
@@ -469,19 +471,19 @@ static uint32_t
shard_blob_key(VCL_BLOB key_blob)
{
uint8_t k[4] = { 0 };
- uint8_t *b;
+ const uint8_t *b;
int i, ki;
- assert(key_blob);
+ AN(key_blob);
+ AN(key_blob->blob);
assert(key_blob->len > 0);
- assert(key_blob->priv != NULL);
if (key_blob->len >= 4)
ki = 0;
else
ki = 4 - key_blob->len;
- b = key_blob->priv;
+ b = key_blob->blob;
for (i = 0; ki < 4; i++, ki++)
k[ki] = b[i];
assert(i <= key_blob->len);
@@ -570,7 +572,7 @@ shard_param_args(VRT_CTX,
return (NULL);
}
if (key_blob == NULL || key_blob->len <= 0 ||
- key_blob->priv == NULL) {
+ key_blob->blob == NULL) {
sharddir_err(ctx, SLT_Error, "%s %s: "
"by=BLOB but no or empty key_blob "
"- using key 0",
@@ -1014,12 +1016,17 @@ vmod_shard_param_get_healthy(VRT_CTX,
}
static const struct vmod_directors_shard_param *
-shard_param_blob(const VCL_BLOB blob)
+shard_param_blob(VCL_BLOB blob)
{
- if (blob && blob->priv &&
- blob->len == sizeof(struct vmod_directors_shard_param) &&
- *(unsigned *)blob->priv == VMOD_SHARD_SHARD_PARAM_MAGIC)
- return (blob->priv);
+ const struct vmod_directors_shard_param *p;
+
+ if (blob && blob->type == VMOD_SHARD_SHARD_PARAM_BLOB &&
+ blob->blob != NULL &&
+ blob->len == sizeof(struct vmod_directors_shard_param)) {
+ CAST_OBJ_NOTNULL(p, blob->blob, VMOD_SHARD_SHARD_PARAM_MAGIC);
+ return (p);
+ }
+
return (NULL);
}
@@ -1027,20 +1034,9 @@ VCL_BLOB v_matchproto_(td_directors_shard_param_use)
vmod_shard_param_use(VRT_CTX,
struct vmod_directors_shard_param *p)
{
- struct vmod_priv *blob;
-
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
CHECK_OBJ_NOTNULL(p, VMOD_SHARD_SHARD_PARAM_MAGIC);
- blob = (void *)WS_Alloc(ctx->ws, sizeof *blob);
- if (blob == NULL) {
- VRT_fail(ctx, "Workspace overflow (param.use())");
- return (NULL);
- }
-
- memset(blob, 0, sizeof *blob);
- blob->len = sizeof *p;
- blob->priv = p;
-
- return (blob);
+ return (VRT_blob(ctx, "xshard_param.use()", p, sizeof *p,
+ VMOD_SHARD_SHARD_PARAM_BLOB));
}
More information about the varnish-commit
mailing list