summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNicholas Mc Guire <hofrat@osadl.org>2016-10-08 17:51:45 +0200
committerMichal Marek <mmarek@suse.com>2016-10-11 09:57:17 +0200
commitc8990359d4b12f14656386526ddf904635076902 (patch)
tree40be3d2b71d8e7b2156e7f8249f3a4104680b259
parent1e01892e7ad521a96760da0d791b42badf755d3f (diff)
downloadop-kernel-dev-c8990359d4b12f14656386526ddf904635076902.zip
op-kernel-dev-c8990359d4b12f14656386526ddf904635076902.tar.gz
Coccinelle: flag conditions with no effect
Report code constructs where the if and else branch are functionally identical. In cases where this is intended it really should be documented - most reported cases probably are bugs. Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org> Signed-off-by: Michal Marek <mmarek@suse.com>
-rw-r--r--scripts/coccinelle/misc/cond_no_effect.cocci64
1 files changed, 64 insertions, 0 deletions
diff --git a/scripts/coccinelle/misc/cond_no_effect.cocci b/scripts/coccinelle/misc/cond_no_effect.cocci
new file mode 100644
index 0000000..8467dbd
--- /dev/null
+++ b/scripts/coccinelle/misc/cond_no_effect.cocci
@@ -0,0 +1,64 @@
+///Find conditions where if and else branch are functionally
+// identical.
+//
+// There can be false positives in cases where the positional
+// information is used (as with lockdep) or where the identity
+// is a placeholder for not yet handled cases.
+// Unfortunately there also seems to be a tendency to use
+// the last if else/else as a "default behavior" - which some
+// might consider a legitimate coding pattern. From discussion
+// on kernelnewbies though it seems that this is not really an
+// accepted pattern and if at all it would need to be commented
+//
+// In the Linux kernel it does not seem to actually report
+// false positives except for those that were documented as
+// being intentional.
+// the two known cases are:
+// arch/sh/kernel/traps_64.c:read_opcode()
+// } else if ((pc & 1) == 0) {
+// /* SHcompact */
+// /* TODO : provide handling for this. We don't really support
+// user-mode SHcompact yet, and for a kernel fault, this would
+// have to come from a module built for SHcompact. */
+// return -EFAULT;
+// } else {
+// /* misaligned */
+// return -EFAULT;
+// }
+// fs/kernfs/file.c:kernfs_fop_open()
+// * Both paths of the branch look the same. They're supposed to
+// * look that way and give @of->mutex different static lockdep keys.
+// */
+// if (has_mmap)
+// mutex_init(&of->mutex);
+// else
+// mutex_init(&of->mutex);
+//
+// All other cases look like bugs or at least lack of documentation
+//
+// Confidence: Moderate
+// Copyright: (C) 2016 Nicholas Mc Guire, OSADL. GPLv2.
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual org
+virtual report
+
+@cond@
+statement S1;
+position p;
+@@
+
+* if@p (...) S1 else S1
+
+@script:python depends on org@
+p << cond.p;
+@@
+
+cocci.print_main("WARNING: possible condition with no effect (if == else)",p)
+
+@script:python depends on report@
+p << cond.p;
+@@
+
+coccilib.report.print_report(p[0],"WARNING: possible condition with no effect (if == else)")
OpenPOWER on IntegriCloud