PVS-Studio: analyzing wxWidgets code

Do you like to promote your wxWidgets based application or component!? Post it here and let's see what the critics have to say. Also, if you found that ONE wx component the world needs to know about, put it here for future reference.
Post Reply
Andrey_Karpov
In need of some credit
In need of some credit
Posts: 1
Joined: Tue Feb 28, 2012 4:58 pm

PVS-Studio: analyzing wxWidgets code

Post by Andrey_Karpov »

Hello, All.

We are regularly asked to check various open-source projects with
the PVS-Studio analyzer. If you want to offer some project for us
to analyze too, please follow this link: http://www.viva64.com/en/b/0124/

The new project we have checked is wxWidgets.

We have checked the project using the PVS-Studio v4.54 analyzer:
http://www.viva64.com/en/pvs-studio/

The trial version of PVS-Studio can be used to thoroughly analyze
the wxWidgets project: it will be sufficient to review all the odd code
fragments. We will cite here only code fragments that caught our glance.

Additional. Free license for free open-source software:
http://www.viva64.com/en/b/0092/

ISSUES :

Code: Select all

-----------------------------------------------------------
V501 There are identical sub-expressions 'ch == '/'' to the left and to the right of the '||' operator. wxscintilla lexmmixal.cxx 37

inline bool isMMIXALOperator(char ch) {
 if (isalnum(ch))
  return false;
 if (ch == '+' || ch == '-' || ch == '|' || ch == '^' ||
  ch == '*' || ch == '/' || ch == '/' ||
  ch == '%' || ch == '<' || ch == '>' || ch == '&' ||
  ch == '~' || ch == '$' ||
  ch == ',' || ch == '(' || ch == ')' ||
  ch == '[' || ch == ']')
  return true;
 return false;
}
-----------------------------------------------------------
V530 The return value of function 'empty' is required to be utilized. base translation.cpp 989

bool wxMsgCatalogFile::LoadData(....)
{
  ...
  wxString m_charset;
  ...
  if ( m_charset == wxS("CHARSET") )
  {
    m_charset.empty();
  }
  ...
}
-----------------------------------------------------------
V530 The return value of function 'empty' is required to be utilized. base registry.cpp 329

wxString m_strKey; 

void wxRegKey::SetHkey(WXHKEY hKey)
{
  ...

  // reset old data
  m_strKey.empty();
  m_dwLastError = 0;
}
-----------------------------------------------------------
V560 A part of conditional expression is always true: 20. wxscintilla lextads3.cxx 701

#define SCE_T3_OPERATOR 5
#define SCE_T3_BRACE 20

static inline bool IsAnOperator(const int style) {
  return style == SCE_T3_OPERATOR || SCE_T3_BRACE;
}
-----------------------------------------------------------
V595 The 'm_art' pointer was utilized before it was verified against nullptr. Check lines: 2659, 2664. aui auibar.cpp 2659

void wxAuiToolBar::OnRightDown(wxMouseEvent& evt)
{
  ...
  if (m_overflowSizerItem)
  {
    int dropdown_size = m_art->GetElementSize(wxAUI_TBART_OVERFLOW_SIZE);
    if (dropdown_size > 0 &&
      evt.m_x > cli_rect.width - dropdown_size &&
      evt.m_y >= 0 &&
      evt.m_y < cli_rect.height &&
      m_art)
    {
       return;
    }
  }
  ...
}

And here:
V595 The 'm_art' pointer was utilized before it was verified against nullptr. Check lines: 2726, 2731. aui auibar.cpp 2726
-----------------------------------------------------------
V513 Use _beginthreadex/_endthreadex functions instead of CreateThread/ExitThread functions. base utilsexc.cpp 961
V513 Use _beginthreadex/_endthreadex functions instead of CreateThread/ExitThread functions. core dialup.cpp 1139
-----------------------------------------------------------
V519 The 'm_isDirty' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 249, 250. core dragimgg.cpp 250

bool m_isDirty;
bool m_isShown;

bool wxGenericDragImage::BeginDrag(const wxPoint& hotspot,
                                   wxWindow* window,
                                   bool fullScreen,
                                   wxRect* rect)
{
  ...
  m_isDirty = false;
  m_isDirty = false;
  ...
}

We think that maybe written so:
m_isDirty = false;
m_isShown = false;
-----------------------------------------------------------
V519 The 'rasDialParams.szCallbackNumber[0]' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 832, 833. core dialup.cpp 833

WCHAR szEntryName[ RAS_MaxEntryName + 1 ];
WCHAR szPhoneNumber[ RAS_MaxPhoneNumber + 1 ];
WCHAR szCallbackNumber[ RAS_MaxCallbackNumber + 1 ];

bool wxDialUpManagerMSW::Dial(const wxString& nameOfISP,
                              const wxString& username,
                              const wxString& password,
                              bool async)
{
  ...
  // default values for other fields
  rasDialParams.szPhoneNumber[0] = '\0';
  rasDialParams.szCallbackNumber[0] = '\0';
  rasDialParams.szCallbackNumber[0] = '\0';
  ...
}

We think that maybe written so:
rasDialParams.szEntryName[0] = '\0';
rasDialParams.szPhoneNumber[0] = '\0';
rasDialParams.szCallbackNumber[0] = '\0';
-----------------------------------------------------------
Strange...

V572 It is odd that the object which was created using 'new' operator is immediately casted to another type. base appbase.cpp 812

class wxFontMapper : public wxFontMapperBase
{
  ...
}

wxFontMapper *wxConsoleAppTraitsBase::CreateFontMapper()
{
  return (wxFontMapper *)new wxFontMapperBase;
}

And here:
V572 It is odd that the object which was created using 'new' operator is immediately casted to another type. base fmapbase.cpp 428
-----------------------------------------------------------
V592 The expression was enclosed by parentheses twice: ((expression)). One pair of parentheses is unnecessary or misprint is present. wxscintilla lexau3.cxx 781

static void FoldAU3Doc(....)
{
  ...
  // if a keyword is found on the current line and the line doesn't end with _ (continuation)
  //    and we are not inside a commentblock.
  if (szKeywordlen > 0 && (!(chPrev == '_')) && 
      ((!(IsStreamCommentStyle(style)) || foldInComment)) ) {
  ...
}

Strange...
We think that maybe written so:

if (szKeywordlen > 0 && (!(chPrev == '_')) && 
    (!(IsStreamCommentStyle(style) || foldInComment)) ) {

And here:
V592 The expression was enclosed by parentheses twice: ((expression)). One pair of parentheses is unnecessary or misprint is present. wxscintilla lexpowerpro.cxx 466
-----------------------------------------------------------
V526 The 'strcmp' function returns 0 if corresponding strings are equal. Consider examining the condition for mistakes. wxscintilla lexerlang.cxx 204

static void ColouriseErlangDoc(....) {
{
  ...
  } else if (erlangBIFs.InList(cur)
             && strcmp(cur,"erlang:")){
    style = SCE_ERLANG_BIFS;
  }
  ...
}

We think that maybe written so:
strcmp(cur,"erlang:") == 0

In other files write: strcmp() == / != 0.
-----------------------------------------------------------
P.S. Welcome to Twitter: @Code_Analysis
DavidHart
Site Admin
Site Admin
Posts: 4252
Joined: Thu Jan 12, 2006 6:23 pm
Location: IoW, UK

Re: PVS-Studio: analyzing wxWidgets code

Post by DavidHart »

Hi,

Thank you for your post, which I've moved to a more-appropriate place.

This is a user forum, so the details of your findings may be of less interest here than to those on the wx-dev mailing list; where you (very sensibly) also posted.

Regards,

David
Post Reply