LibRaw, Coverity SCAN, PVS-Studio

    LibRaw and PVS-Studio
    I read a note on checking a small LibRaw project using Coverity SCAN. It follows from the article that nothing interesting was found. I decided to try whether PVS-Studio analyzer can find something.

    Libaw


    LibRaw is a library for reading RAW files received from digital cameras (CRW / CR2, NEF, RAF, DNG and others). Website: http://www.libraw.org/

    Verification with Coverity SCAN


    And here is an article that inspired me to check the project using PVS-Studio: " About static analysis of C ++ ". I will briefly quote the main part of the article:

    Coverity SCAN: 107 warnings, of which about a third are with High Impact.

    From High Impact:

    Pieces 10 in Microsoft STL

    Still some of this kind of look:
    int variable;
    if(layout==Layout1) variable=value1;
    if(layout==Layout2) variable=value2;

    And a warning is given to this, they say inaccurately, not an initialized variable. I agree with him on the general feelings, so do not do this. But in real life there are two types of layout - and this is clearly stated in the calling code. Those. the machine has enough data to realize that this is not a 'High impact', but just inaccurate.

    A certain number of warnings, saying unsigned short when expanding to 32-64 bits, can hurt a bite. I don’t want to argue with this - formally the machine is right, but in fact the dimensions of the picture live in these unsigned short and they will not grow up to 32767 in the coming years.

    Those. again, there is no need to fix it - in the case of this subject area.

    All other problems found by 'High Impact' are simply false positives. Those. the code, I agree, is not perfect (you should have seen this code from dcraw!), but everything found is not an error.

    Validation with PVS-Studio


    Now let's see if PVS-Studio analyzer can find something after Coverity. Of course, there are no expectations about finding super errors, but it's still interesting to try.

    PVS-Studio analyzer generated 46 general-purpose warnings (first and second severity level).

    I suggest looking at the code snippets that seemed interesting to me.

    Typos


    void DHT::hide_hots() {
      ....
      for (int k = -2; k < 3; k += 2)
        for (int m = -2; m < 3; m += 2)
          if (m == 0 && m == 0)
            continue;
          else
            avg += nraw[nr_offset(y + k, x + m)][kc];
      ....
    }

    Warning PVS-Studio: V501 There are identical sub-expressions to the left and to the right of the '&&' operator: m == 0 && m == 0 dht_demosaic.cpp 260

    Apparently we are dealing with a typo. Most likely, the check should have been like this:
    if (k == 0 && m == 0)

    An identical fragment is also in the aahd_demosaic.cpp file (line 199).

    Priority Operations


    int main(int argc, char *argv[])
    {
      int ret;
      ....
      if( (ret = RawProcessor.open_buffer(iobuffer,st.st_size)
                 != LIBRAW_SUCCESS))
      {
        fprintf(stderr,"Cannot open_buffer %s: %s\n",
          argv[arg],libraw_strerror(ret));
        free(iobuffer);
        continue;
      }
      ....
    }

    PVS-Studio Warning: V593 Consider reviewing the expression of the 'A = B! = C' kind. The expression is calculated as following: 'A = (B! = C)'. dcraw_emu.cpp 468

    Error related to operation priorities. At the beginning, the comparison “RawProcessor.open_buffer (iobuffer, st.st_size)! = LIBRAW_SUCCESS” is performed. Then the result of this comparison is written to the variable 'ret'. If an error occurs, the wrong error code will be printed to the file. Not a critical flaw, but still worth talking about it.

    Negative shift


    unsigned CLASS pana_bits (int nbits)
    {
      .... 
      return (buf[byte] | buf[byte+1] << 8) >>
             (vbits & 7) & ~(-1 << nbits);
      ....
    }

    Warning PVS-Studio: V610 Undefined behavior. Check the shift operator '<<. The left operand '-1' is negative. dcraw_common.cpp 1827

    Shifting negative values ​​results in undefined behavior. These tricks are often used, and the program pretends to work. But in fact, you cannot rely on such code. You can read more about the shifts of negative numbers here: Without knowing the ford, do not get into the water. Part three .

    Similar shifts can be found here:
    • dcraw_common.cpp 1851
    • dcraw_common.cpp 2085
    • dcraw_common.cpp 2814
    • dcraw_common.cpp 6644

    Strange fragments


    void DHT::illustrate_dline(int i) {
      ....
        int l = ndir[nr_offset(y, x)] & 8;
        l >>= 3;
        l = 1;
      ....
    }

    PVS-Studio Warning: V519 The 'l' variable is assigned values ​​twice successively. Perhaps this is a mistake. Check lines: 671, 672. dht_demosaic.cpp 672

    Perhaps this is not a mistake and "l = 1" is written specifically. However, the code looks suspicious.

    Here is another suspicious place:
    void CLASS identify()
    {
      ....
      if (!load_raw && (maximum = 0xfff))
      ....
    }

    PVS-Studio warning: V560 A part of conditional expression is always true: ((imgdata.color.maximum) = 0xfff). dcraw_common.cpp 8496

    Conclusion


    Both analyzers found very little. This is natural for small projects. However, it was still interesting to conduct an experiment to test LibRaw.

    Also popular now: