Hi, In this evening's offline self-hackathon in the train I came up with a macro for a recurring pattern in Varnish Cache that I also happen to use in at least one VMOD so it might benefit others too.
I used Coccinelle (and still love it) in order to apply the change, so if the first patch is accepted you remember doing this kind of pointer acquisition in one of your VMOD or VUT, I've attached the semantic patch too. If the name "acquire" is not appropriate (because I know at least one other meaning) please know that I'm short on naming ideas. Best, Dridi
From f8dacfce348ff3a3fec2683fb4fb06a2d76cc6a2 Mon Sep 17 00:00:00 2001 From: Dridi Boukelmoune <[email protected]> Date: Sun, 13 Mar 2016 18:53:06 +0100 Subject: [PATCH 1/2] Add a miniobj macro for pointer acquisitions There's a recurring pattern throughout the code base of acquiring a reference (by nulling the original) essentially when resources are freed. Instead of repeating this anti dangling pointers measure, it can be wrapped in a documenting utility macro. --- bin/varnishd/cache/cache_busyobj.c | 5 +---- include/miniobj.h | 8 ++++++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c index c3fa436..b41b5da 100644 --- a/bin/varnishd/cache/cache_busyobj.c +++ b/bin/varnishd/cache/cache_busyobj.c @@ -76,10 +76,7 @@ vbo_Free(struct busyobj **bop) { struct busyobj *bo; - AN(bop); - bo = *bop; - *bop = NULL; - CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC); + ACQUIRE_OBJ_NOTNULL(bo, bop, BUSYOBJ_MAGIC); AZ(bo->htc); MPL_Free(vbopool, bo); } diff --git a/include/miniobj.h b/include/miniobj.h index 8b4f043..6cbe1cf 100644 --- a/include/miniobj.h +++ b/include/miniobj.h @@ -59,6 +59,14 @@ CHECK_OBJ((to), (type_magic)); \ } while (0) +#define ACQUIRE_OBJ_NOTNULL(to, pfrom, type_magic) \ + do { \ + assert((pfrom) != NULL); \ + (to) = *(pfrom); \ + *(pfrom) = NULL; \ + CHECK_OBJ_NOTNULL((to), (type_magic)); \ + } while (0) + #define REPLACE(ptr, val) \ do { \ free(ptr); \ -- 2.5.0
From 2dadb93d0b510313d80a2254e5a9d3c41491bbc5 Mon Sep 17 00:00:00 2001 From: Dridi Boukelmoune <[email protected]> Date: Sun, 13 Mar 2016 18:54:21 +0100 Subject: [PATCH 2/2] Use the ACQUIRE_OBJ_NOTNULL macro where relevant This patch was created using Coccinelle and the following steps: $ cat >acquire.cocci <<EOF @@ expression t, f; constant m; @@ - AN(f); - t = *f; - *f = NULL; - CHECK_OBJ_NOTNULL(t, m); + ACQUIRE_OBJ_NOTNULL(t, f, m); EOF $ spatch --sp-file acquire.cocci --dir . --in-place --- bin/varnishd/cache/cache_backend_cfg.c | 5 +---- bin/varnishd/cache/cache_backend_tcp.c | 5 +---- bin/varnishd/cache/cache_busyobj.c | 5 +---- bin/varnishd/cache/cache_hash.c | 5 +---- bin/varnishd/cache/cache_mempool.c | 5 +---- bin/varnishd/cache/cache_obj.c | 5 +---- bin/varnishd/cache/cache_vrt_vmod.c | 5 +---- bin/varnishd/common/common_vsm.c | 6 +----- bin/varnishd/storage/storage_lru.c | 5 +---- bin/varnishd/waiter/cache_waiter.c | 5 +---- lib/libvarnishapi/vsl_dispatch.c | 11 ++--------- lib/libvarnishapi/vsl_query.c | 5 +---- lib/libvarnishapi/vxp.c | 5 +---- lib/libvmod_directors/vdir.c | 6 +----- 14 files changed, 15 insertions(+), 63 deletions(-) diff --git a/bin/varnishd/cache/cache_backend_cfg.c b/bin/varnishd/cache/cache_backend_cfg.c index 21f0782..a00f816 100644 --- a/bin/varnishd/cache/cache_backend_cfg.c +++ b/bin/varnishd/cache/cache_backend_cfg.c @@ -148,10 +148,7 @@ VRT_delete_backend(VRT_CTX, struct director **dp) struct backend *be; CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); - AN(dp); - d = *dp; - *dp = NULL; - CHECK_OBJ_NOTNULL(d, DIRECTOR_MAGIC); + ACQUIRE_OBJ_NOTNULL(d, dp, DIRECTOR_MAGIC); CAST_OBJ_NOTNULL(be, d->priv, BACKEND_MAGIC); Lck_Lock(&be->mtx); be->admin_health = vbe_ah_deleted; diff --git a/bin/varnishd/cache/cache_backend_tcp.c b/bin/varnishd/cache/cache_backend_tcp.c index b43bf3c..f039a9c 100644 --- a/bin/varnishd/cache/cache_backend_tcp.c +++ b/bin/varnishd/cache/cache_backend_tcp.c @@ -178,10 +178,7 @@ VBT_Rel(struct tcp_pool **tpp) struct tcp_pool *tp; struct vbc *vbc, *vbc2; - AN(tpp); - tp = *tpp; - *tpp = NULL; - CHECK_OBJ_NOTNULL(tp, TCP_POOL_MAGIC); + ACQUIRE_OBJ_NOTNULL(tp, tpp, TCP_POOL_MAGIC); assert(tp->refcnt > 0); if (--tp->refcnt > 0) return; diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c index b41b5da..ca177d2 100644 --- a/bin/varnishd/cache/cache_busyobj.c +++ b/bin/varnishd/cache/cache_busyobj.c @@ -148,10 +148,7 @@ VBO_ReleaseBusyObj(struct worker *wrk, struct busyobj **pbo) struct busyobj *bo; CHECK_OBJ_ORNULL(wrk, WORKER_MAGIC); - AN(pbo); - bo = *pbo; - *pbo = NULL; - CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC); + ACQUIRE_OBJ_NOTNULL(bo, pbo, BUSYOBJ_MAGIC); CHECK_OBJ_ORNULL(bo->fetch_objcore, OBJCORE_MAGIC); AZ(bo->htc); diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c index 21088f7..f61cd98 100644 --- a/bin/varnishd/cache/cache_hash.c +++ b/bin/varnishd/cache/cache_hash.c @@ -860,10 +860,7 @@ HSH_DerefObjHead(struct worker *wrk, struct objhead **poh) int r; CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); - AN(poh); - oh = *poh; - *poh = NULL; - CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC); + ACQUIRE_OBJ_NOTNULL(oh, poh, OBJHEAD_MAGIC); if (oh == private_oh) { assert(VTAILQ_EMPTY(&oh->waitinglist)); diff --git a/bin/varnishd/cache/cache_mempool.c b/bin/varnishd/cache/cache_mempool.c index f4c1c30..6869233 100644 --- a/bin/varnishd/cache/cache_mempool.c +++ b/bin/varnishd/cache/cache_mempool.c @@ -253,10 +253,7 @@ MPL_Destroy(struct mempool **mpp) { struct mempool *mpl; - AN(mpp); - mpl = *mpp; - *mpp = NULL; - CHECK_OBJ_NOTNULL(mpl, MEMPOOL_MAGIC); + ACQUIRE_OBJ_NOTNULL(mpl, mpp, MEMPOOL_MAGIC); Lck_Lock(&mpl->mtx); AZ(mpl->live); mpl->self_destruct = 1; diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c index 46fd0fc..ca83be9 100644 --- a/bin/varnishd/cache/cache_obj.c +++ b/bin/varnishd/cache/cache_obj.c @@ -161,10 +161,7 @@ ObjDestroy(struct worker *wrk, struct objcore **p) struct objcore *oc; CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); - AN(p); - oc = *p; - *p = NULL; - CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); + ACQUIRE_OBJ_NOTNULL(oc, p, OBJCORE_MAGIC); if (oc->boc != NULL) obj_deleteboc(&oc->boc); FREE_OBJ(oc); diff --git a/bin/varnishd/cache/cache_vrt_vmod.c b/bin/varnishd/cache/cache_vrt_vmod.c index 50e2d61..1fa6ed0 100644 --- a/bin/varnishd/cache/cache_vrt_vmod.c +++ b/bin/varnishd/cache/cache_vrt_vmod.c @@ -149,10 +149,7 @@ VRT_Vmod_Fini(struct vmod **hdl) ASSERT_CLI(); - AN(hdl); - v = *hdl; - *hdl = NULL; - CHECK_OBJ_NOTNULL(v, VMOD_MAGIC); + ACQUIRE_OBJ_NOTNULL(v, hdl, VMOD_MAGIC); #ifndef DONT_DLCLOSE_VMODS /* diff --git a/bin/varnishd/common/common_vsm.c b/bin/varnishd/common/common_vsm.c index 0dda841..422f92e 100644 --- a/bin/varnishd/common/common_vsm.c +++ b/bin/varnishd/common/common_vsm.c @@ -337,11 +337,7 @@ VSM_common_delete(struct vsm_sc **scp) struct vsm_range *vr, *vr2; struct vsm_sc *sc; - AN(scp); - sc =*scp; - *scp = NULL; - - CHECK_OBJ_NOTNULL(sc, VSM_SC_MAGIC); + ACQUIRE_OBJ_NOTNULL(sc, scp, VSM_SC_MAGIC); VTAILQ_FOREACH_SAFE(vr, &sc->r_free, list, vr2) FREE_OBJ(vr); VTAILQ_FOREACH_SAFE(vr, &sc->r_used, list, vr2) diff --git a/bin/varnishd/storage/storage_lru.c b/bin/varnishd/storage/storage_lru.c index 785e3e1..d63ffaa 100644 --- a/bin/varnishd/storage/storage_lru.c +++ b/bin/varnishd/storage/storage_lru.c @@ -71,10 +71,7 @@ LRU_Free(struct lru **pp) { struct lru *lru; - AN(pp); - lru = *pp; - *pp = NULL; - CHECK_OBJ_NOTNULL(lru, LRU_MAGIC); + ACQUIRE_OBJ_NOTNULL(lru, pp, LRU_MAGIC); Lck_Lock(&lru->mtx); AN(VTAILQ_EMPTY(&lru->lru_head)); Lck_Unlock(&lru->mtx); diff --git a/bin/varnishd/waiter/cache_waiter.c b/bin/varnishd/waiter/cache_waiter.c index b1d0801..df5fa66 100644 --- a/bin/varnishd/waiter/cache_waiter.c +++ b/bin/varnishd/waiter/cache_waiter.c @@ -178,10 +178,7 @@ Waiter_Destroy(struct waiter **wp) { struct waiter *w; - AN(wp); - w = *wp; - *wp = NULL; - CHECK_OBJ_NOTNULL(w, WAITER_MAGIC); + ACQUIRE_OBJ_NOTNULL(w, wp, WAITER_MAGIC); AZ(binheap_root(w->heap)); AN(w->impl->fini); diff --git a/lib/libvarnishapi/vsl_dispatch.c b/lib/libvarnishapi/vsl_dispatch.c index bbcd7cc..018b40c 100644 --- a/lib/libvarnishapi/vsl_dispatch.c +++ b/lib/libvarnishapi/vsl_dispatch.c @@ -526,11 +526,7 @@ vtx_retire(struct VSLQ *vslq, struct vtx **pvtx) struct chunk *chunk; AN(vslq); - AN(pvtx); - - vtx = *pvtx; - *pvtx = NULL; - CHECK_OBJ_NOTNULL(vtx, VTX_MAGIC); + ACQUIRE_OBJ_NOTNULL(vtx, pvtx, VTX_MAGIC); AN(vtx->flags & VTX_F_COMPLETE); AN(vtx->flags & VTX_F_READY); @@ -1109,10 +1105,7 @@ VSLQ_Delete(struct VSLQ **pvslq) struct VSLQ *vslq; struct vtx *vtx; - AN(pvslq); - vslq = *pvslq; - *pvslq = NULL; - CHECK_OBJ_NOTNULL(vslq, VSLQ_MAGIC); + ACQUIRE_OBJ_NOTNULL(vslq, pvslq, VSLQ_MAGIC); (void)VSLQ_Flush(vslq, NULL, NULL); AZ(vslq->n_outstanding); diff --git a/lib/libvarnishapi/vsl_query.c b/lib/libvarnishapi/vsl_query.c index b2e0860..8e16200 100644 --- a/lib/libvarnishapi/vsl_query.c +++ b/lib/libvarnishapi/vsl_query.c @@ -332,10 +332,7 @@ vslq_deletequery(struct vslq_query **pquery) { struct vslq_query *query; - AN(pquery); - query = *pquery; - *pquery = NULL; - CHECK_OBJ_NOTNULL(query, VSLQ_QUERY_MAGIC); + ACQUIRE_OBJ_NOTNULL(query, pquery, VSLQ_QUERY_MAGIC); AN(query->vex); vex_Free(&query->vex); diff --git a/lib/libvarnishapi/vxp.c b/lib/libvarnishapi/vxp.c index db91b30..b285177 100644 --- a/lib/libvarnishapi/vxp.c +++ b/lib/libvarnishapi/vxp.c @@ -183,10 +183,7 @@ vxp_Delete(struct vxp **pvxp) struct vxp *vxp; struct membit *mb; - AN(pvxp); - vxp = *pvxp; - *pvxp = NULL; - CHECK_OBJ_NOTNULL(vxp, VXP_MAGIC); + ACQUIRE_OBJ_NOTNULL(vxp, pvxp, VXP_MAGIC); while (!VTAILQ_EMPTY(&vxp->membits)) { mb = VTAILQ_FIRST(&vxp->membits); diff --git a/lib/libvmod_directors/vdir.c b/lib/libvmod_directors/vdir.c index 15dd56a..9d5b439 100644 --- a/lib/libvmod_directors/vdir.c +++ b/lib/libvmod_directors/vdir.c @@ -81,11 +81,7 @@ vdir_delete(struct vdir **vdp) { struct vdir *vd; - AN(vdp); - vd = *vdp; - *vdp = NULL; - - CHECK_OBJ_NOTNULL(vd, VDIR_MAGIC); + ACQUIRE_OBJ_NOTNULL(vd, vdp, VDIR_MAGIC); free(vd->backend); free(vd->weight); -- 2.5.0
acquire.cocci
Description: Binary data
_______________________________________________ varnish-dev mailing list [email protected] https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
