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

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();
 	}
 }

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);
 }

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)

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)
 {
 

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);
 }

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);
 }

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);
 }

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);
 }

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);
 }