[master] 10b79a439 Add past Coccinelle patches to the collection
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Tue Apr 16 08:08:08 UTC 2019
commit 10b79a439ae211bdd386e03afab6c1a795d818a4
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Tue Apr 16 10:02:46 2019 +0200
Add past Coccinelle patches to the collection
Slightly enhanced compared to when they were first used. I ran them and
found more opportunities to polish the code base.
diff --git a/bin/varnishd/common/common_vsmw.c b/bin/varnishd/common/common_vsmw.c
index 4e54a4aea..ba83b862b 100644
--- a/bin/varnishd/common/common_vsmw.c
+++ b/bin/varnishd/common/common_vsmw.c
@@ -158,7 +158,7 @@ vsmw_mkent(const struct vsmw *vsmw, const char *pfx)
if (fd < 0 && errno == ENOENT)
return;
if (fd >= 0)
- AZ(close(fd));
+ closefd(&fd);
}
}
@@ -178,7 +178,7 @@ vsmw_addseg(struct vsmw *vsmw, struct vsmwseg *seg)
AZ(VSB_finish(vsmw->vsb));
s = write(fd, VSB_data(vsmw->vsb), VSB_len(vsmw->vsb));
assert(s == VSB_len(vsmw->vsb));
- AZ(close(fd));
+ closefd(&fd);
}
/*--------------------------------------------------------------------*/
@@ -212,7 +212,7 @@ vsmw_delseg(struct vsmw *vsmw, struct vsmwseg *seg, int fixidx)
AZ(VSB_finish(vsmw->vsb));
s = write(fd, VSB_data(vsmw->vsb), VSB_len(vsmw->vsb));
assert(s == VSB_len(vsmw->vsb));
- AZ(close(fd));
+ closefd(&fd);
AZ(renameat(vsmw->vdirfd, t, vsmw->vdirfd, vsmw->idx));
REPLACE(t, NULL);
}
@@ -250,7 +250,7 @@ vsmw_newcluster(struct vsmw *vsmw, size_t len, const char *pfx)
MAP_HASSEMAPHORE | MAP_NOSYNC | MAP_SHARED,
fd, 0);
- AZ(close(fd));
+ closefd(&fd);
assert(vc->ptr != MAP_FAILED);
(void)mlock(vc->ptr, len);
@@ -286,10 +286,7 @@ VSMW_DestroyCluster(struct vsmw *vsmw, struct vsmw_cluster **vsmcp)
vsmw_lock();
CHECK_OBJ_NOTNULL(vsmw, VSMW_MAGIC);
- AN(vsmcp);
- vc = *vsmcp;
- *vsmcp = NULL;
- CHECK_OBJ_NOTNULL(vc, VSMW_CLUSTER_MAGIC);
+ TAKE_OBJ_NOTNULL(vc, vsmcp, VSMW_CLUSTER_MAGIC);
if (vc->cseg != NULL) {
/*
@@ -430,7 +427,7 @@ VSMW_New(int vdirfd, int mode, const char *idxname)
vsmw->idx, O_APPEND | O_WRONLY | O_CREAT, vsmw->mode);
assert(fd >= 0);
vsmw_idx_head(vsmw, fd);
- AZ(close(fd));
+ closefd(&fd);
vsmw_unlock();
return (vsmw);
@@ -450,7 +447,7 @@ VSMW_Destroy(struct vsmw **pp)
assert (errno == ENOENT);
REPLACE(vsmw->idx, NULL);
VSB_destroy(&vsmw->vsb);
- AZ(close(vsmw->vdirfd));
+ closefd(&vsmw->vdirfd);
FREE_OBJ(vsmw);
vsmw_unlock();
}
diff --git a/lib/libvarnish/vjsn.c b/lib/libvarnish/vjsn.c
index 2ef5629ca..6a265cbe0 100644
--- a/lib/libvarnish/vjsn.c
+++ b/lib/libvarnish/vjsn.c
@@ -95,10 +95,7 @@ vjsn_delete(struct vjsn **jp)
{
struct vjsn *js;
- AN(jp);
- js = *jp;
- *jp = NULL;
- CHECK_OBJ_NOTNULL(js, VJSN_MAGIC);
+ TAKE_OBJ_NOTNULL(js, jp, VJSN_MAGIC);
if (js->value != NULL)
vjsn_val_delete(js->value);
free(js->raw);
diff --git a/lib/libvarnish/vlu.c b/lib/libvarnish/vlu.c
index bc32df9e0..d7e163616 100644
--- a/lib/libvarnish/vlu.c
+++ b/lib/libvarnish/vlu.c
@@ -77,10 +77,7 @@ VLU_Destroy(struct vlu **lp)
{
struct vlu *l;
- AN(lp);
- l = *lp;
- *lp = NULL;
- CHECK_OBJ_NOTNULL(l, LINEUP_MAGIC);
+ TAKE_OBJ_NOTNULL(l, lp, LINEUP_MAGIC);
free(l->buf);
FREE_OBJ(l);
}
diff --git a/tools/coccinelle/check_obj.cocci b/tools/coccinelle/check_obj.cocci
new file mode 100644
index 000000000..37eff6b68
--- /dev/null
+++ b/tools/coccinelle/check_obj.cocci
@@ -0,0 +1,13 @@
+/*
+ * This patch removes a redundant null check.
+ */
+
+@@
+expression obj, magic;
+@@
+
+if (obj != NULL) {
+- CHECK_OBJ_NOTNULL(obj, magic);
++ CHECK_OBJ(obj, magic);
+...
+}
diff --git a/tools/coccinelle/closefd.cocci b/tools/coccinelle/closefd.cocci
new file mode 100644
index 000000000..f1f75a33a
--- /dev/null
+++ b/tools/coccinelle/closefd.cocci
@@ -0,0 +1,24 @@
+/*
+ * The closefd() macro takes a pointer to the file descriptor to wipe it in
+ * addition to closing the file and checking there was no error. This can
+ * prevent use-after-close bugs. It will not work if the variable or field
+ * holding the file descriptor has a const qualifier. It also make sures that
+ * fd looks like a valid file descriptor before even calling close().
+ *
+ * The second part of the patch undoes the change when fd is a constant such
+ * as STDIN_FILENO where it becomes nonsensical.
+ */
+
+@@
+expression fd;
+@@
+
+- AZ(close(fd));
++ closefd(&fd);
+
+@@
+constant fd;
+@@
+
+- closefd(&fd);
++ AZ(close(fd));
diff --git a/tools/coccinelle/free_obj.cocci b/tools/coccinelle/free_obj.cocci
new file mode 100644
index 000000000..13feaf74d
--- /dev/null
+++ b/tools/coccinelle/free_obj.cocci
@@ -0,0 +1,27 @@
+/*
+ * Make sure to use FREE_OBJ() instead of a plain free() to get additional
+ * safeguards offered by the macro.
+ */
+
+@@
+expression obj, objp, magic;
+@@
+
+(
+ALLOC_OBJ(obj, magic);
+|
+CAST_OBJ(obj, objp, magic);
+|
+CAST_OBJ_NOTNULL(obj, objp, magic);
+|
+CHECK_OBJ(obj, magic);
+|
+CHECK_OBJ_NOTNULL(obj, magic);
+|
+CHECK_OBJ_ORNULL(obj, magic);
+|
+TAKE_OBJ_NOTNULL(obj, objp, magic);
+)
+...
+- free(obj);
++ FREE_OBJ(obj);
diff --git a/tools/coccinelle/take_obj_notnull.cocci b/tools/coccinelle/take_obj_notnull.cocci
new file mode 100644
index 000000000..98ac7ca93
--- /dev/null
+++ b/tools/coccinelle/take_obj_notnull.cocci
@@ -0,0 +1,39 @@
+/*
+ * The TAKE_OBJ_NOTNULL() macro emulates move semantics and better conveys the
+ * intent behind a common pattern in the code base, usually before freeing an
+ * object.
+ *
+ * This may fail to capture other incarnations of this pattern where the order
+ * of the operations is not exactly followed so we try several combinations.
+ */
+
+@@
+expression obj, objp, magic;
+@@
+
+- AN(objp);
+- obj = *objp;
+- *objp = NULL;
+- CHECK_OBJ_NOTNULL(obj, magic);
++ TAKE_OBJ_NOTNULL(obj, objp, magic);
+
+@@
+expression obj, objp, magic;
+@@
+
+- AN(objp);
+- obj = *objp;
+- CHECK_OBJ_NOTNULL(obj, magic);
+- *objp = NULL;
++ TAKE_OBJ_NOTNULL(obj, objp, magic);
+
+@@
+expression obj, objp, magic;
+@@
+
+- AN(objp);
+- obj = *objp;
+- CHECK_OBJ_NOTNULL(obj, magic);
++ TAKE_OBJ_NOTNULL(obj, objp, magic);
+...
+- *objp = NULL;
More information about the varnish-commit
mailing list