[master] 3cd00428b Generalize TAKE_OBJ_NOTNULL in object destructors
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Mon Mar 18 22:35:08 UTC 2019
commit 3cd00428b7a9886d3e23cc119d9cde14079d2149
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Mon Mar 18 23:27:32 2019 +0100
Generalize TAKE_OBJ_NOTNULL in object destructors
It is now safe to assume that a constructor is always called with proper
values or never called to begin with, and as such to panic if it's not
the case.
Bonus polish in the files I visited.
Refs #2297
diff --git a/bin/varnishtest/tests/m00000.vtc b/bin/varnishtest/tests/m00000.vtc
index 45262850b..5ddd706ce 100644
--- a/bin/varnishtest/tests/m00000.vtc
+++ b/bin/varnishtest/tests/m00000.vtc
@@ -123,9 +123,17 @@ varnish v1 -errvcl {Expression has type STRING, expected REAL} {
varnish v1 -errvcl {Failed initialization} {
import debug;
+ import directors;
backend default { .host = "127.0.0.1"; }
sub vcl_init {
return (fail);
+ # uninitialized objects coverage
new xyz = debug.obj();
+ new fb = directors.fallback();
+ new hsh = directors.hash();
+ new rnd = directors.random();
+ new rr = directors.round_robin();
+ new shd = directors.shard();
+ new shp = directors.shard_param();
}
}
diff --git a/lib/libvmod_blob/vmod_blob.c b/lib/libvmod_blob/vmod_blob.c
index 7820e810c..2f8686edf 100644
--- a/lib/libvmod_blob/vmod_blob.c
+++ b/lib/libvmod_blob/vmod_blob.c
@@ -301,16 +301,13 @@ vmod_blob__fini(struct vmod_blob_blob **blobp)
{
struct vmod_blob_blob *b;
- if (blobp == NULL || *blobp == NULL)
- return;
+ TAKE_OBJ_NOTNULL(b, blobp, VMOD_BLOB_MAGIC);
- b = *blobp;
- *blobp = NULL;
- CHECK_OBJ(b, VMOD_BLOB_MAGIC);
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++) {
char *s = b->encoding[i][j];
@@ -319,6 +316,7 @@ vmod_blob__fini(struct vmod_blob_blob **blobp)
b->encoding[i][j] = NULL;
}
}
+
AZ(pthread_mutex_destroy(&b->lock));
FREE_OBJ(b);
}
diff --git a/lib/libvmod_debug/vmod_debug_dyn.c b/lib/libvmod_debug/vmod_debug_dyn.c
index c7026b4ea..e8085394e 100644
--- a/lib/libvmod_debug/vmod_debug_dyn.c
+++ b/lib/libvmod_debug/vmod_debug_dyn.c
@@ -144,21 +144,16 @@ xyzzy_dyn__init(VRT_CTX, struct xyzzy_debug_dyn **dynp,
*dynp = dyn;
}
-VCL_VOID
+VCL_VOID v_matchproto_(td_xyzzy_debug_dyn__fini)
xyzzy_dyn__fini(struct xyzzy_debug_dyn **dynp)
{
struct xyzzy_debug_dyn *dyn;
- AN(dynp);
- if (*dynp == NULL)
- return; /* failed initialization */
-
- CAST_OBJ_NOTNULL(dyn, *dynp, VMOD_DEBUG_DYN_MAGIC);
+ TAKE_OBJ_NOTNULL(dyn, dynp, VMOD_DEBUG_DYN_MAGIC);
/* at this point all backends will be deleted by the vcl */
free(dyn->vcl_name);
AZ(pthread_mutex_destroy(&dyn->mtx));
FREE_OBJ(dyn);
- *dynp = NULL;
}
VCL_BACKEND v_matchproto_()
@@ -194,6 +189,11 @@ dyn_uds_init(VRT_CTX, struct xyzzy_debug_dyn_uds *uds, VCL_STRING path)
VRT_fail(ctx, "path must be an absolute path: %s", path);
return (-1);
}
+
+ /* XXX: now that we accept that sockets may come after vcl.load
+ * and change during the life of a VCL, do we still need to check
+ * this? It looks like both if blocks can be retired.
+ */
errno = 0;
if (stat(path, &st) != 0) {
VRT_fail(ctx, "Cannot stat path %s: %s", path, strerror(errno));
@@ -224,9 +224,9 @@ dyn_uds_init(VRT_CTX, struct xyzzy_debug_dyn_uds *uds, VCL_STRING path)
return (0);
}
-VCL_VOID v_matchproto_(td_debug_dyn_uds__init)
+VCL_VOID v_matchproto_(td_xyzzy_debug_dyn_uds__init)
xyzzy_dyn_uds__init(VRT_CTX, struct xyzzy_debug_dyn_uds **udsp,
- const char *vcl_name, VCL_STRING path)
+ const char *vcl_name, VCL_STRING path)
{
struct xyzzy_debug_dyn_uds *uds;
@@ -246,6 +246,7 @@ xyzzy_dyn_uds__init(VRT_CTX, struct xyzzy_debug_dyn_uds **udsp,
FREE_OBJ(uds);
return;
}
+
*udsp = uds;
}
@@ -254,14 +255,10 @@ xyzzy_dyn_uds__fini(struct xyzzy_debug_dyn_uds **udsp)
{
struct xyzzy_debug_dyn_uds *uds;
- if (udsp == NULL || *udsp == NULL)
- return;
- CHECK_OBJ(*udsp, VMOD_DEBUG_UDS_MAGIC);
- uds = *udsp;
+ TAKE_OBJ_NOTNULL(uds, udsp, VMOD_DEBUG_UDS_MAGIC);
free(uds->vcl_name);
AZ(pthread_mutex_destroy(&uds->mtx));
FREE_OBJ(uds);
- *udsp = NULL;
}
VCL_BACKEND v_matchproto_(td_debug_dyn_uds_backend)
diff --git a/lib/libvmod_debug/vmod_debug_obj.c b/lib/libvmod_debug/vmod_debug_obj.c
index 34eacca75..3928e5598 100644
--- a/lib/libvmod_debug/vmod_debug_obj.c
+++ b/lib/libvmod_debug/vmod_debug_obj.c
@@ -38,8 +38,9 @@
struct xyzzy_debug_obj {
unsigned magic;
#define VMOD_DEBUG_OBJ_MAGIC 0xccbd9b77
- int foobar;
- const char *string, *number;
+ int foobar;
+ const char *string;
+ const char *number;
};
VCL_VOID
@@ -61,16 +62,16 @@ xyzzy_obj__init(VRT_CTX, struct xyzzy_debug_obj **op,
AN(*op);
}
-VCL_VOID
+VCL_VOID v_matchproto_(td_xyzzy_obj__fini)
xyzzy_obj__fini(struct xyzzy_debug_obj **op)
{
+ struct xyzzy_debug_obj *o;
- AN(op);
- AN(*op);
- FREE_OBJ(*op);
+ TAKE_OBJ_NOTNULL(o, op, VMOD_DEBUG_OBJ_MAGIC);
+ FREE_OBJ(o);
}
-VCL_VOID v_matchproto_()
+VCL_VOID v_matchproto_(td_xyzzy_obj_enum)
xyzzy_obj_enum(VRT_CTX, struct xyzzy_debug_obj *o, VCL_ENUM e)
{
diff --git a/lib/libvmod_directors/fall_back.c b/lib/libvmod_directors/fall_back.c
index 0838e93fa..c14e745af 100644
--- a/lib/libvmod_directors/fall_back.c
+++ b/lib/libvmod_directors/fall_back.c
@@ -214,10 +214,6 @@ vmod_fallback__fini(struct vmod_directors_fallback **fbp)
{
struct vmod_directors_fallback *fb;
- // XXX 2297
- if (*fbp == NULL)
- return;
-
TAKE_OBJ_NOTNULL(fb, fbp, VMOD_DIRECTORS_FALLBACK_MAGIC);
VRT_DelDirector(&fb->vd->dir);
}
diff --git a/lib/libvmod_directors/hash.c b/lib/libvmod_directors/hash.c
index 010aaeaa2..a82e19fe3 100644
--- a/lib/libvmod_directors/hash.c
+++ b/lib/libvmod_directors/hash.c
@@ -83,10 +83,6 @@ vmod_hash__fini(struct vmod_directors_hash **rrp)
{
struct vmod_directors_hash *rr;
- // XXX 2297
- if (*rrp == NULL)
- return;
-
TAKE_OBJ_NOTNULL(rr, rrp, VMOD_DIRECTORS_HASH_MAGIC);
VRT_DelDirector(&rr->vd->dir);
}
diff --git a/lib/libvmod_directors/random.c b/lib/libvmod_directors/random.c
index 93e5a96f6..3be74527f 100644
--- a/lib/libvmod_directors/random.c
+++ b/lib/libvmod_directors/random.c
@@ -124,10 +124,6 @@ vmod_random__fini(struct vmod_directors_random **rrp)
{
struct vmod_directors_random *rr;
- // XXX 2297
- if (*rrp == NULL)
- return;
-
TAKE_OBJ_NOTNULL(rr, rrp, VMOD_DIRECTORS_RANDOM_MAGIC);
VRT_DelDirector(&rr->vd->dir);
}
diff --git a/lib/libvmod_directors/round_robin.c b/lib/libvmod_directors/round_robin.c
index fc32ab646..d323f8a5e 100644
--- a/lib/libvmod_directors/round_robin.c
+++ b/lib/libvmod_directors/round_robin.c
@@ -131,10 +131,6 @@ vmod_round_robin__fini(struct vmod_directors_round_robin **rrp)
{
struct vmod_directors_round_robin *rr;
- // XXX 2297
- if (*rrp == NULL)
- return;
-
TAKE_OBJ_NOTNULL(rr, rrp, VMOD_DIRECTORS_ROUND_ROBIN_MAGIC);
VRT_DelDirector(&rr->vd->dir);
}
diff --git a/lib/libvmod_directors/vmod_shard.c b/lib/libvmod_directors/vmod_shard.c
index d7dd7d7d6..c25ebb9ec 100644
--- a/lib/libvmod_directors/vmod_shard.c
+++ b/lib/libvmod_directors/vmod_shard.c
@@ -275,10 +275,6 @@ vmod_shard__fini(struct vmod_directors_shard **vshardp)
{
struct vmod_directors_shard *vshard;
- // XXX 2297
- if (*vshardp == NULL)
- return;
-
TAKE_OBJ_NOTNULL(vshard, vshardp, VMOD_SHARD_SHARD_MAGIC);
VRT_DelDirector(&vshard->dir);
FREE_OBJ(vshard);
@@ -899,10 +895,6 @@ vmod_shard_param__fini(struct vmod_directors_shard_param **pp)
{
struct vmod_directors_shard_param *p;
- // XXX 2297
- if (*pp == NULL)
- return;
-
TAKE_OBJ_NOTNULL(p, pp, VMOD_SHARD_SHARD_PARAM_MAGIC);
FREE_OBJ(p);
}
More information about the varnish-commit
mailing list