Re: [Xen-devel] [PATCH] xen/Coverity: Audit of MISSING_BREAK defects

2015-02-13 Thread Andrew Cooper 
On 12/02/15 21:06, Don Koch wrote:
 On Thu, 12 Feb 2015 20:08:46 +
 Andrew Cooper andrew.coop...@citrix.com wrote:

 Coverity uses several heuristics to identify when one case statement
 legitimately falls through into the next, and a comment as the final item in 
 a
 case statement is one heuristic (the assumption being that it is a
 justification for the fallthrough).

 Use this to perform an audit of defects and hide the legitimate fallthroughs.

 No functional change.  All identified fallthroughs are legitimate.

 Signed-off-by: Andrew Cooper andrew.coop...@citrix.com
 Coverity-IDs: 1055483, 1055484, 1055486 - 1055488, 1055490 - 1055496,
   1055498 - 1055500, 1055501, 1220091
 CC: Keir Fraser k...@xen.org
 CC: Jan Beulich jbeul...@suse.com
 CC: Tim Deegan t...@xen.org
 CC: Xen Coverity Team cover...@xen.org
 ---
  xen/arch/x86/hvm/emulate.c  |1 +
  xen/arch/x86/hvm/svm/svm.c  |1 +
  xen/arch/x86/hvm/vlapic.c   |1 +
  xen/arch/x86/mm.c   |2 ++
  xen/arch/x86/traps.c|3 +++
  xen/arch/x86/x86_64/compat/mm.c |1 +
  xen/common/lib.c|4 
  xen/common/schedule.c   |1 +
  8 files changed, 14 insertions(+)

 diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
 index 636c909..c657bc6 100644
 --- a/xen/arch/x86/hvm/emulate.c
 +++ b/xen/arch/x86/hvm/emulate.c
 @@ -161,6 +161,7 @@ static int hvmemul_do_io(
  put_page(ram_page);
  return X86EMUL_RETRY;
  }
 +/* fallthrough */
  default:
  if ( ram_page )
  put_page(ram_page);
 diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
 index a7655bd..018dd70 100644
 --- a/xen/arch/x86/hvm/svm/svm.c
 +++ b/xen/arch/x86/hvm/svm/svm.c
 @@ -2378,6 +2378,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
  case NESTEDHVM_VMEXIT_ERROR:
  break;
  }
 +/* fallthrough */
  case NESTEDHVM_VMEXIT_ERROR:
  gdprintk(XENLOG_ERR,
  nestedsvm_check_intercepts() returned 
 NESTEDHVM_VMEXIT_ERROR\n);
 diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
 index 5da6d8f..cee8699 100644
 --- a/xen/arch/x86/hvm/vlapic.c
 +++ b/xen/arch/x86/hvm/vlapic.c
 @@ -762,6 +762,7 @@ static int vlapic_reg_write(struct vcpu *v,
  vlapic-hw.tdt_msr = 0;
  }
  vlapic-pt.irq = val  APIC_VECTOR_MASK;
 +/* fallthrough */
  case APIC_LVTTHMR:  /* LVT Thermal Monitor */
  case APIC_LVTPC:/* LVT Performance Counter */
  case APIC_LVT0: /* LVT LINT0 Reg */
 diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
 index d4965da..12e5006 100644
 --- a/xen/arch/x86/mm.c
 +++ b/xen/arch/x86/mm.c
 @@ -2771,6 +2771,7 @@ int new_guest_cr3(unsigned long mfn)
  {
  case -EINTR:
  rc = -ERESTART;
 +/* fallthrough */
  case -ERESTART:
  curr-arch.old_guest_table = page;
  break;
 @@ -3126,6 +3127,7 @@ long do_mmuext_op(
  {
  case -EINTR:
  rc = -ERESTART;
 +/* fallthrough */
  case -ERESTART:
  curr-arch.old_guest_table = page;
  okay = 0;
 diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
 index f5516dc..057a7af 100644
 --- a/xen/arch/x86/traps.c
 +++ b/xen/arch/x86/traps.c
 @@ -1739,7 +1739,9 @@ static int guest_io_okay(
port3, 2) )
  {
  default: x.bytes[0] = ~0;
 +/* fallthrough */
  case 1:  x.bytes[1] = ~0;
 +/* fallthrough */
  case 0:  break;
  }
  TOGGLE_MODE();
 @@ -3320,6 +3322,7 @@ static void pci_serr_error(const struct cpu_user_regs 
 *regs)
  {
  case 'd': /* 'dom0' */
  nmi_hwdom_report(_XEN_NMIREASON_pci_serr);
 +/* fallthrough */
  case 'i': /* 'ignore' */
  /* Would like to print a diagnostic here but can't call printk()
 from NMI context -- raise a softirq instead. */
 diff --git a/xen/arch/x86/x86_64/compat/mm.c 
 b/xen/arch/x86/x86_64/compat/mm.c
 index f90f611..1491ce3 100644
 --- a/xen/arch/x86/x86_64/compat/mm.c
 +++ b/xen/arch/x86/x86_64/compat/mm.c
 @@ -292,6 +292,7 @@ int 
 compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t) cmp_uops,
  break;
  case MMUEXT_NEW_USER_BASEPTR:
  rc = -EINVAL;
 +/* fallthrough */
  case MMUEXT_TLB_FLUSH_LOCAL:
  case MMUEXT_TLB_FLUSH_MULTI:
  case MMUEXT_TLB_FLUSH_ALL:
 diff --git a/xen/common/lib.c b/xen/common/lib.c
 index 89c74ad..ae0bbb3 100644
 --- a/xen/common/lib.c
 +++ b/xen/common/lib.c
 @@ -461,12 +461,16 @@ unsigned long long parse_size_and_unit(const char

Re: [Xen-devel] [PATCH] xen/Coverity: Audit of MISSING_BREAK defects

2015-02-13 Thread Don Koch 
On Fri, 13 Feb 2015 11:01:27 +
Andrew Cooper andrew.coop...@citrix.com wrote:

 On 12/02/15 21:06, Don Koch wrote:
  On Thu, 12 Feb 2015 20:08:46 +
  Andrew Cooper andrew.coop...@citrix.com wrote:
 
[...]
  I'm surprised that coverity didn't complain about the fallthrough in
  the next case:
 
  case TASKLET_scheduled:
  clear_bit(_TASKLET_scheduled, tasklet_work);
  case 0:
  /*tasklet_work_scheduled = 0;*/
  break;
 
  Or is my code out of date?
 
  With or without that change,
  Reviewed-by: Don Koch dk...@verizon.com
 
 Another heuristic is a case statement followed by another case which
 begins with a break.  The defect would re-emerge if code gets uncommented.

Gotcha. My R-b still stands.

Thanks,
-d

 ~Andrew
 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] xen/Coverity: Audit of MISSING_BREAK defects

2015-02-12 Thread Andrew Cooper 
Coverity uses several heuristics to identify when one case statement
legitimately falls through into the next, and a comment as the final item in a
case statement is one heuristic (the assumption being that it is a
justification for the fallthrough).

Use this to perform an audit of defects and hide the legitimate fallthroughs.

No functional change.  All identified fallthroughs are legitimate.

Signed-off-by: Andrew Cooper andrew.coop...@citrix.com
Coverity-IDs: 1055483, 1055484, 1055486 - 1055488, 1055490 - 1055496,
  1055498 - 1055500, 1055501, 1220091
CC: Keir Fraser k...@xen.org
CC: Jan Beulich jbeul...@suse.com
CC: Tim Deegan t...@xen.org
CC: Xen Coverity Team cover...@xen.org
---
 xen/arch/x86/hvm/emulate.c  |1 +
 xen/arch/x86/hvm/svm/svm.c  |1 +
 xen/arch/x86/hvm/vlapic.c   |1 +
 xen/arch/x86/mm.c   |2 ++
 xen/arch/x86/traps.c|3 +++
 xen/arch/x86/x86_64/compat/mm.c |1 +
 xen/common/lib.c|4 
 xen/common/schedule.c   |1 +
 8 files changed, 14 insertions(+)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 636c909..c657bc6 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -161,6 +161,7 @@ static int hvmemul_do_io(
 put_page(ram_page);
 return X86EMUL_RETRY;
 }
+/* fallthrough */
 default:
 if ( ram_page )
 put_page(ram_page);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index a7655bd..018dd70 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2378,6 +2378,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
 case NESTEDHVM_VMEXIT_ERROR:
 break;
 }
+/* fallthrough */
 case NESTEDHVM_VMEXIT_ERROR:
 gdprintk(XENLOG_ERR,
 nestedsvm_check_intercepts() returned 
NESTEDHVM_VMEXIT_ERROR\n);
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 5da6d8f..cee8699 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -762,6 +762,7 @@ static int vlapic_reg_write(struct vcpu *v,
 vlapic-hw.tdt_msr = 0;
 }
 vlapic-pt.irq = val  APIC_VECTOR_MASK;
+/* fallthrough */
 case APIC_LVTTHMR:  /* LVT Thermal Monitor */
 case APIC_LVTPC:/* LVT Performance Counter */
 case APIC_LVT0: /* LVT LINT0 Reg */
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index d4965da..12e5006 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2771,6 +2771,7 @@ int new_guest_cr3(unsigned long mfn)
 {
 case -EINTR:
 rc = -ERESTART;
+/* fallthrough */
 case -ERESTART:
 curr-arch.old_guest_table = page;
 break;
@@ -3126,6 +3127,7 @@ long do_mmuext_op(
 {
 case -EINTR:
 rc = -ERESTART;
+/* fallthrough */
 case -ERESTART:
 curr-arch.old_guest_table = page;
 okay = 0;
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index f5516dc..057a7af 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1739,7 +1739,9 @@ static int guest_io_okay(
   port3, 2) )
 {
 default: x.bytes[0] = ~0;
+/* fallthrough */
 case 1:  x.bytes[1] = ~0;
+/* fallthrough */
 case 0:  break;
 }
 TOGGLE_MODE();
@@ -3320,6 +3322,7 @@ static void pci_serr_error(const struct cpu_user_regs 
*regs)
 {
 case 'd': /* 'dom0' */
 nmi_hwdom_report(_XEN_NMIREASON_pci_serr);
+/* fallthrough */
 case 'i': /* 'ignore' */
 /* Would like to print a diagnostic here but can't call printk()
from NMI context -- raise a softirq instead. */
diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c
index f90f611..1491ce3 100644
--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -292,6 +292,7 @@ int 
compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t) cmp_uops,
 break;
 case MMUEXT_NEW_USER_BASEPTR:
 rc = -EINVAL;
+/* fallthrough */
 case MMUEXT_TLB_FLUSH_LOCAL:
 case MMUEXT_TLB_FLUSH_MULTI:
 case MMUEXT_TLB_FLUSH_ALL:
diff --git a/xen/common/lib.c b/xen/common/lib.c
index 89c74ad..ae0bbb3 100644
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -461,12 +461,16 @@ unsigned long long parse_size_and_unit(const char *s, 
const char **ps)
 {
 case 'T': case 't':
 ret = 10;
+/* fallthrough */
 case 'G': case 'g':
 ret = 10;
+/* fallthrough */
 case 'M': case 'm':
 ret = 10;
+/* fallthrough */

Re: [Xen-devel] [PATCH] xen/Coverity: Audit of MISSING_BREAK defects

2015-02-12 Thread Don Koch 
On Thu, 12 Feb 2015 20:08:46 +
Andrew Cooper andrew.coop...@citrix.com wrote:

 Coverity uses several heuristics to identify when one case statement
 legitimately falls through into the next, and a comment as the final item in a
 case statement is one heuristic (the assumption being that it is a
 justification for the fallthrough).
 
 Use this to perform an audit of defects and hide the legitimate fallthroughs.
 
 No functional change.  All identified fallthroughs are legitimate.
 
 Signed-off-by: Andrew Cooper andrew.coop...@citrix.com
 Coverity-IDs: 1055483, 1055484, 1055486 - 1055488, 1055490 - 1055496,
   1055498 - 1055500, 1055501, 1220091
 CC: Keir Fraser k...@xen.org
 CC: Jan Beulich jbeul...@suse.com
 CC: Tim Deegan t...@xen.org
 CC: Xen Coverity Team cover...@xen.org
 ---
  xen/arch/x86/hvm/emulate.c  |1 +
  xen/arch/x86/hvm/svm/svm.c  |1 +
  xen/arch/x86/hvm/vlapic.c   |1 +
  xen/arch/x86/mm.c   |2 ++
  xen/arch/x86/traps.c|3 +++
  xen/arch/x86/x86_64/compat/mm.c |1 +
  xen/common/lib.c|4 
  xen/common/schedule.c   |1 +
  8 files changed, 14 insertions(+)
 
 diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
 index 636c909..c657bc6 100644
 --- a/xen/arch/x86/hvm/emulate.c
 +++ b/xen/arch/x86/hvm/emulate.c
 @@ -161,6 +161,7 @@ static int hvmemul_do_io(
  put_page(ram_page);
  return X86EMUL_RETRY;
  }
 +/* fallthrough */
  default:
  if ( ram_page )
  put_page(ram_page);
 diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
 index a7655bd..018dd70 100644
 --- a/xen/arch/x86/hvm/svm/svm.c
 +++ b/xen/arch/x86/hvm/svm/svm.c
 @@ -2378,6 +2378,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
  case NESTEDHVM_VMEXIT_ERROR:
  break;
  }
 +/* fallthrough */
  case NESTEDHVM_VMEXIT_ERROR:
  gdprintk(XENLOG_ERR,
  nestedsvm_check_intercepts() returned 
 NESTEDHVM_VMEXIT_ERROR\n);
 diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
 index 5da6d8f..cee8699 100644
 --- a/xen/arch/x86/hvm/vlapic.c
 +++ b/xen/arch/x86/hvm/vlapic.c
 @@ -762,6 +762,7 @@ static int vlapic_reg_write(struct vcpu *v,
  vlapic-hw.tdt_msr = 0;
  }
  vlapic-pt.irq = val  APIC_VECTOR_MASK;
 +/* fallthrough */
  case APIC_LVTTHMR:  /* LVT Thermal Monitor */
  case APIC_LVTPC:/* LVT Performance Counter */
  case APIC_LVT0: /* LVT LINT0 Reg */
 diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
 index d4965da..12e5006 100644
 --- a/xen/arch/x86/mm.c
 +++ b/xen/arch/x86/mm.c
 @@ -2771,6 +2771,7 @@ int new_guest_cr3(unsigned long mfn)
  {
  case -EINTR:
  rc = -ERESTART;
 +/* fallthrough */
  case -ERESTART:
  curr-arch.old_guest_table = page;
  break;
 @@ -3126,6 +3127,7 @@ long do_mmuext_op(
  {
  case -EINTR:
  rc = -ERESTART;
 +/* fallthrough */
  case -ERESTART:
  curr-arch.old_guest_table = page;
  okay = 0;
 diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
 index f5516dc..057a7af 100644
 --- a/xen/arch/x86/traps.c
 +++ b/xen/arch/x86/traps.c
 @@ -1739,7 +1739,9 @@ static int guest_io_okay(
port3, 2) )
  {
  default: x.bytes[0] = ~0;
 +/* fallthrough */
  case 1:  x.bytes[1] = ~0;
 +/* fallthrough */
  case 0:  break;
  }
  TOGGLE_MODE();
 @@ -3320,6 +3322,7 @@ static void pci_serr_error(const struct cpu_user_regs 
 *regs)
  {
  case 'd': /* 'dom0' */
  nmi_hwdom_report(_XEN_NMIREASON_pci_serr);
 +/* fallthrough */
  case 'i': /* 'ignore' */
  /* Would like to print a diagnostic here but can't call printk()
 from NMI context -- raise a softirq instead. */
 diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c
 index f90f611..1491ce3 100644
 --- a/xen/arch/x86/x86_64/compat/mm.c
 +++ b/xen/arch/x86/x86_64/compat/mm.c
 @@ -292,6 +292,7 @@ int 
 compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(mmuext_op_compat_t) cmp_uops,
  break;
  case MMUEXT_NEW_USER_BASEPTR:
  rc = -EINVAL;
 +/* fallthrough */
  case MMUEXT_TLB_FLUSH_LOCAL:
  case MMUEXT_TLB_FLUSH_MULTI:
  case MMUEXT_TLB_FLUSH_ALL:
 diff --git a/xen/common/lib.c b/xen/common/lib.c
 index 89c74ad..ae0bbb3 100644
 --- a/xen/common/lib.c
 +++ b/xen/common/lib.c
 @@ -461,12 +461,16 @@ unsigned long long parse_size_and_unit(const char *s, 
 const char **ps)
  {