Once again, the PVS-Studio analyzer was more attentive than a human

    Take the bug

    Studying the warnings of the PVS-Studio analyzer in the process of checking various open projects, we see again and again how useful this tool can be. The code analyzer is incredibly attentive and never tired. It indicates errors that escape even with a careful review of the code. Consider another such case.

    The last time I wrote a similar note , studying the source code of the StarEngine project: 2D Game Engine. Now the analyzer has shown its superiority over me during the Qt framework testing.

    The last time we checked the Qt framework in 2014. Much time has passed, the project has changed, and many new diagnostics have appeared in the PVS-Studio analyzer. So, it is possible to write another article, which I did.

    Writing out interesting examples of errors, I met this code:

    QWindowsCursor::CursorState QWindowsCursor::cursorState()
{
  enum { cursorShowing = 0x1, cursorSuppressed = 0x2 };
  CURSORINFO cursorInfo;
  cursorInfo.cbSize = sizeof(CURSORINFO);
  if (GetCursorInfo(&cursorInfo)) {
    if (cursorInfo.flags & CursorShowing)   // <= V616
  ....
}

    For this code, PVS-Studio issued a warning:

    V616 CWE-480 is used in the bitwise operation. qwindowscursor.cpp 669

    The unstable version of PVS-Studio was used for the verification, so my faith in the analyzer faltered. “Eh, we broke something in the processing mechanisms of unnamed transfers,” I sighed, and wrote this case to the bugtracker as an error, leading to a false positive.

    I was absolutely sure that the analyzer was mistaken. After all, just a few lines above it says that the constant CursorShowing is equal to 1.

    At the same time I tried to be attentive! I looked through the code several times to make sure the analyzer was wrong. I designed this code snippet and the corresponding message as an error in the bugtracker.

    I did a careful review of this little bit of code, and I still messed up. The analyzer is right, not the person.

    In a detailed study of the situation, it turned out that the named constant cursorShowing is declared above , and the condition uses the CursorShowing constant . The difference is only in the first letter! In one place it is lowercase, and in another capital.

    Why is the code compiled? Because the constant CursorShowing also exists. Here is her announcement:

    classQWindowsCursor :public QPlatformCursor
{
public:
  enum CursorState {
    CursorShowing,
    CursorHidden,
    CursorSuppressed
  };
  ....
}

    As you can see, the CursorShowing constant is equal to 0. Therefore, the PVS-Studio analyzer is absolutely right to say that the condition (cursorInfo.flags & CursorShowing) does not make sense. The condition is always false.

    The analyzer found a wonderful typo. Love static code analysis! :)


    If you want to share this article with an English-speaking audience, then please use the link to the translation: Andrey Karpov. Once again, the PVS-Studio analyzer has been more attentive than a person .
    Tags:

    Also popular now: