ccode-analysisstatic-code-analysiscoverity

How can I disable coverity checking using code annotation?


There is a problem, coverity finds an error (potential OoB) in the place of the code where semantically this problem cannot arise. Because of the static analyzer’s message, I don’t want to make an additional check for this place in the blueprint. Is it possible to suppress this warning with code annotation?

static struct rule *Rule_sort(struct rule *rp){
  int i;
  struct rule *pNext;
  struct rule *x[32];
  memset(x, 0, sizeof(x));
  while( rp ){
    pNext = rp->next;
    rp->next = 0;
    for(i=0; i<sizeof(x)/sizeof(x[0]) && x[i]; i++){
      rp = Rule_merge(x[i], rp);
      x[i] = 0;
    }
    // potential OoB
    x[i] = rp;
    rp = pNext;
  }
  rp = 0;
  for(i=0; i<sizeof(x)/sizeof(x[0]); i++){
    rp = Rule_merge(x[i], rp);
  }
  return rp;
}

Solution

  • To suppress a Coverty finding with a source code annotation, add a comment to the line just before where the finding is reported of the form // coverity[event_tag] or /* coverity[event_tag] */, where event_tag is the "tag" of the event. The tag is an identifier-like word that indicates the general form of that event. See the blog post Coverity: Suppressing false positives with code annotations for a few more details.

    You haven't shown the Coverity finding, but I suspect it is an OVERRUN and the tag is overrun-local. Assuming so, you could suppress it like this:

        for(i=0; i<sizeof(x)/sizeof(x[0]) && x[i]; i++){
          rp = Rule_merge(x[i], rp);
          x[i] = 0;
        }
        /* coverity[overrun-local] */
        x[i] = rp;
        rp = pNext;
    

    With this annotation, the next time the analysis runs and its results committed to the database, the finding will be automatically marked "Intentional", and hence it will not appear in the default list of findings or other common reports.

    If you're not sure where to locate the tag for a finding, see How do I get the event tags for a Coverity issue?


    Having explained how to suppress the finding, in this particular case I would advise against it. I think the code quoted in the question is not safe, as the inner for loop condition i<sizeof(x)/sizeof(x[0]) seems to be wrong. That is equivalent to i < 32 here. If that condition could ever be false, it would mean i was equal to 32, and hence the loop would terminate with i==32, and therefore overrun the array by one element.

    Put another way, if you ever get into the inner for loop while i==31, the array will be overrun because i will then always be incremented to 32 and x[i] written.


    Even if there was nothing wrong per se with the code, I'd still be hesitant to simply suppress the finding. Think of a static analysis finding like a concern raised by a colleague: even if the concern is not indicative of a bug, take it as an opportunity to improve the clarity and robustness of the code. In a case like this, if the code was correct but Coverity complained, I would lean towards adding an assert rather than outright suppression.