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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 12/02/15 20:49, Don Koch wrote:
> On Thu, 12 Feb 2015 20:08:33 +0000
> Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 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.
>>
>> There are two bugfixes identified in the audit, both minor:
>>  * 'n' command line handling for gtracestat
>>  * BKSPC handling in xentop
>>
>> All other identified defaults are legitimate fallthoughs
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Coverity-IDs: 1055464, 1055465, 1055467, 1055468, 1055481, 1055482
>> CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
>> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
>> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
>> CC: Xen Coverity Team <coverity@xxxxxxx>
>> ---
>>  tools/libxl/xl_cmdimpl.c         |    3 +++
>>  tools/misc/gtracestat.c          |    1 +
>>  tools/misc/gtraceview.c          |    1 +
>>  tools/xenstat/xentop/xentop.c    |    1 +
>>  tools/xenstore/xenstore_client.c |    1 +
>>  5 files changed, 7 insertions(+)
>>
>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> index 440db78..53c16eb 100644
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -2752,11 +2752,14 @@ static int64_t parse_mem_size_kb(const char *mem)
>>      switch (tolower((uint8_t)*endptr)) {
>>      case 't':
>>          kbytes <<= 10;
>> +        /* fallthrough */
>>      case 'g':
>>          kbytes <<= 10;
>> +        /* fallthrough */
>>      case '\0':
>>      case 'm':
>>          kbytes <<= 10;
>> +        /* fallthrough */
>>      case 'k':
>>          break;
>>      case 'b':
>> diff --git a/tools/misc/gtracestat.c b/tools/misc/gtracestat.c
>> index 874a043..a59e536 100644
>> --- a/tools/misc/gtracestat.c
>> +++ b/tools/misc/gtracestat.c
>> @@ -167,6 +167,7 @@ int main(int argc, char *argv[])
>>              tsc2phase = atoll(optarg);
>>              if (tsc2phase <= 0)
>>                  tsc2phase = 55800000UL;
>> +            break;
>>          case 'd':
>>              is_digest = 1;
>>              break;
>> diff --git a/tools/misc/gtraceview.c b/tools/misc/gtraceview.c
>> index cf9287c..501f86a 100644
>> --- a/tools/misc/gtraceview.c
>> +++ b/tools/misc/gtraceview.c
>> @@ -1097,6 +1097,7 @@ void choose_cpus(void)
>>                      this->init();
>>                  return;
>>              }
>> +            /* fallthrough */
>>          case KEY_F(4):
>>              exit(EXIT_SUCCESS);
>>          }
>> diff --git a/tools/xenstat/xentop/xentop.c b/tools/xenstat/xentop/xentop.c
>> index 3062cb5..23b57f1 100644
>> --- a/tools/xenstat/xentop/xentop.c
>> +++ b/tools/xenstat/xentop/xentop.c
>> @@ -407,6 +407,7 @@ static int handle_key(int ch)
>>              case KEY_BACKSPACE:
>>                      if(prompt_val_len > 0)
>>                              prompt_val[--prompt_val_len] = '\0';
>> +                        break;
> Whitespace? (Yeah, inconsistent tools dir coding style. :-P )
>
> Otherwise...
> Reviewed-by: Don Koch <dkoch@xxxxxxxxxxx>

Urgh - I thought I had fixed all of that.  (I did specifically try)

I can resubmit, or it could be fixed on commit, at the preference of the
committer.

~Andrew

>
>>              default:
>>                      if((prompt_val_len+1) < PROMPT_VAL_LEN
>>                         && isprint(ch)) {
>> diff --git a/tools/xenstore/xenstore_client.c 
>> b/tools/xenstore/xenstore_client.c
>> index 1054f18..3d14d37 100644
>> --- a/tools/xenstore/xenstore_client.c
>> +++ b/tools/xenstore/xenstore_client.c
>> @@ -87,6 +87,7 @@ usage(enum mode mode, int incl_mode, const char *progname)
>>      errx(1, "Usage: %s %s[-h] [-s] [-t] key [...]", progname, mstr);
>>      case MODE_exists:
>>      mstr = incl_mode ? "exists " : "";
>> +    /* fallthrough */
>>      case MODE_list:
>>      mstr = mstr ? : incl_mode ? "list " : "";
>>      errx(1, "Usage: %s %s[-h] [-p] [-s] key [...]", progname, mstr);
>> -- 
>> 1.7.10.4
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxx
>> http://lists.xen.org/xen-devel
>>


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