Menu

#182 PVS-Studio

None
open
nobody
None
1
2015-02-22
2013-09-05
No

I checked TinyCAD, by using PVS-Studio (http://www.viva64.com/). I found only few interesting things. Nevertheless, I decided to write. Perhaps it will be helpful to you.

-------------------------------------------------------------------------------
V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '+' operator. ruler.cpp 66

BOOL hasOrigin;

void Ruler::OnNewSize(CRect nSize)
{
  ....
  Size = CRect(Size.left + hasOrigin ? RULER_WIDTH : 0,
               Size.top, Size.right, Size.top + RULER_WIDTH);
  ....
}
-------------------------------------------------------------------------------
V541 It is dangerous to print the string 'csText' into itself. childfrm.cpp 79

void CChildFrame::OnUpdateFrameTitle(BOOL bAddToTitle)
{
  ....
  CString csText;
  ....
  if (m_nWindow > 0)
    csText.Format(_T("%s: %d"), csText, m_nWindow);
  ....
}
-------------------------------------------------------------------------------
V547 Expression 'parentPos >= 0' is always true. Unsigned type value is always >= 0. bomgenerator.cpp 77

void CBOMGenerator::GenerateBomForDesign(
  int level, size_t parentPos, ....)
{
  ....
  if (parentPos >= 0) cmrParent = cmrDefaultParent;
  ....
}
-------------------------------------------------------------------------------
V576 Incorrect format. A different number of actual arguments is expected while calling 'Format' function. Expected: 2. Present: 3. option.cpp 266
V576 Incorrect format. A different number of actual arguments is expected while calling 'Format' function. Expected: 2. Present: 3. option.cpp 270

CString COption::PointToDisplay(CDPoint a, BOOL horiz)
{
  ....
  if (horiz)
  {
    r.Format(_T("%9.03f"), sy,
             GetUnits() == 0 ? _T("mm") : _T("\""));
  }
  else
  {
    r.Format(_T("%9.03f"), sx, 
             GetUnits() == 0 ? _T("mm") : _T("\""));
  }
  ....
}
-------------------------------------------------------------------------------
V595 The 'i' pointer was utilized before it was verified against nullptr. Check lines: 142, 144. drawmetafile.cpp 142

bool CDrawMetaFile::setImageFile(const TCHAR *filename)
{
  CImage *i = NULL;

  ....

  // Read in the buffer
  i->SetCompressedData(buffer, bytes);

  if (i != NULL)
  {
    m_metafile = m_pDesign->GetOptions()->AddMetaFile(i);
  }

  return i != NULL;
}
-------------------------------------------------------------------------------
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] Np;'. context.cpp 446

void CContext::PolyBezier(CDPoint *pts, int Size)
{
  // Make a new array to hold the points
  POINT *Np = new POINT[Size];
  ....
  delete Np;
}

And:
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] pLogPal;'. imagepng.cpp 213
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] buf;'. streamdb.cpp 182
-------------------------------------------------------------------------------
V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 86, 90. paint.cpp 86

void CTinyCadView::OnDraw(CDC* pDC)
{
  ....
  // Is any of this region in the off-page area?
  if (!pDC->IsPrinting())
  {
    // Paint the region white
    if (pDC->IsPrinting())
    {
      dc.SelectBrush(cWHITE);
      dc.SelectPen(PS_SOLID, 1, cWHITE);
    }
  ....
}
-------------------------------------------------------------------------------
V636 The 'iwidth / actual_width[i]' expression was implicitly casted from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. context.cpp 712

void CContext::TextOut(....)
{
  int *actual_width = new int[len];
  double max_adjust = max_char_adjust;
  int iwidth;
  ....
  if (iwidth / actual_width[i] < 1.0 - max_adjust)
  ....
}

And:
V636 The 'iwidth / actual_width[i]' expression was implicitly casted from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. context.cpp 717
V636 The '3 / 2' expression was implicitly casted from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. drawjunction.cpp 179
V636 The '3 / 2' expression was implicitly casted from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. drawjunction.cpp 185
V636 The '5 / 2' expression was implicitly casted from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. drawmethod.cpp 1284
V636 The '5 / 2' expression was implicitly casted from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. drawmethod.cpp 1301
V636 The '5 / 2' expression was implicitly casted from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. editdlghierarchicaledit.cpp 513
V636 The '5 / 2' expression was implicitly casted from 'int' type to 'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. editdlgmethodedit.cpp 561
-------------------------------------------------------------------------------
V668 There is no sense in testing the 'SortArray' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. imagepng.cpp 935

bool CImagePNG::SubdivColorMap(....)
{
  ....
  if ((SortArray = new QuantizedColorType*[....]) == NULL)
    return ERROR;
  ....
}

And:
V668 There is no sense in testing the 'lpBMIH' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. imagepng.cpp 162
-------------------------------------------------------------------------------

Andrey Karpov.

Related

Bugs: #182

Discussion

  • Don Lucas

    Don Lucas - 2013-09-05

    Hi Andrey -

    Thanks for your efforts in analyzing the TinyCAD source. Which source tree did you analyze? Hopefully you analyzed from the SVN trunk?

    I have been trying to remove all of these types of problems this past year. I will take a look at this and incorporate your suggestions.

    I looked around at your web site and was very impressed. I was not aware of your product previously. I couldn't talk you into running a 64 bit conversion analysis of TinyCAD, could I? I have cleaned up all of the kinds of issues that I know about, but there is much that I do not know about and I haven't checked out the 3rd party libraries. I don't even know if Microsoft supports MFC libraries in a 64 bit environment. TinyCAD was originally written between 16 and 20 years ago. I have been maintaining it for the past 5 years or so, but the original author is no longer available to work on it. I have performed extensive lint checking using only the Visual C++ compiler with the warning levels set to the max and cleaned up all of the problems that I found. I recently updated the NSIS installer to install properly on 32 bit or 64 bit systems, although that was mostly trivial (note - the installer is committed, but not released yet).

    Best regards,
    Don Lucas

     
  • Andrey Karpov

    Andrey Karpov - 2013-09-06

    Hello, Don

    To be honest, I don't know if that was a trunk or stable version. My
    colleague checked a lot of projects about a month ago, and now I
    gradually sort them out when I have time. If there are few bugs in a
    project, I just send a report to the developers - that was the case with
    TinyCAD. When there are quite many bugs, I'd prefer to write a blog-post
    to report on them, which is meant to serve as an advertisement for
    PVS-Studio. I asked my colleague which version he had checked, but he
    didn't remember.

    However, it doesn't matter that much which version was checked. I
    recommend that you check the project yourself. I am not familiar with
    logic of other projects; besides, I have to examine the warning list
    quite superficially. You see, it's only me alone to do this work, and I
    have many projects to study. If you want, I can generate a free
    registration key for you for one week so that you could check the
    project. In addition, you'll have the chance to check if you have any
    issues related to 64 bits in your projects.

    If you like PVS-Studio, we can choose among a number of ways of
    cooperation. For example, we can grant you a key for a long time if you
    mention on the SourceForge website that you used our analyzer to
    maintain TinyCAD; or publish a small post describing our tool. We try to
    cooperate with developers of Open-Source projects, and it's a usual
    practice for us to provide them with free keys. But I hope you can
    understand our policy, too. We can't afford simply giving keys right and
    left. We want people to reward us for that somehow. Let it be a
    blog-post about our tool, a twitter-message mentioning that they used
    PVS-Studio -- whatever. Too few developers know about us, so we try to
    promote the tool and make the word "PVS-Studio" recognizable.


    Best regards,
    Andrey Karpov
    Cand. Sc. (Physics and Mathematics), CTO
    OOO "Program Verification Systems" (Co Ltd)
    URL: www.viva64.com
    E-Mail: karpov@viva64.com

    On 05.09.2013 21:54, Don Lucas wrote:

    Hi Andrey -

    Thanks for your efforts in analyzing the TinyCAD source. Which source
    tree did you analyze? Hopefully you analyzed from the SVN trunk?

    I have been trying to remove all of these types of problems this past
    year. I will take a look at this and incorporate your suggestions.

    I looked around at your web site and was very impressed. I was not
    aware of your product previously. I couldn't talk you into running a
    64 bit conversion analysis of TinyCAD, could I? I have cleaned up all
    of the kinds of issues that I know about, but there is much that I do
    not know about and I haven't checked out the 3rd party libraries. I
    don't even know if Microsoft supports MFC libraries in a 64 bit
    environment. TinyCAD was originally written between 16 and 20 years
    ago. I have been maintaining it for the past 5 years or so, but the
    original author is no longer available to work on it. I have performed
    extensive lint checking using only the Visual C++ compiler with the
    warning levels set to the max and cleaned up all of the problems that
    I found. I recently updated the NSIS installer to install properly on
    32 bit or 64 bit systems, although that was mostly trivial (note - the
    installer is committed, but not released yet).

    Best regards,
    Don Lucas


    [bugs:#182] http://sourceforge.net/p/tinycad/bugs/182/ PVS-Studio

    Status: open
    Created: Thu Sep 05, 2013 04:31 PM UTC by Andrey Karpov
    Last Updated: Thu Sep 05, 2013 04:31 PM UTC
    Owner: nobody

    I checked TinyCAD, by using PVS-Studio (http://www.viva64.com/). I
    found only few interesting things. Nevertheless, I decided to write.
    Perhaps it will be helpful to you.


    V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '+' operator. ruler.cpp 66

    BOOL hasOrigin;

    void Ruler::OnNewSize(CRect nSize)
    {
    ....
    Size = CRect(Size.left + hasOrigin ? RULER_WIDTH : 0,
    Size.top, Size.right, Size.top + RULER_WIDTH);
    ....
    }


    V541 It is dangerous to print the string 'csText' into itself. childfrm.cpp 79

    void CChildFrame::OnUpdateFrameTitle(BOOL bAddToTitle)
    {
    ....
    CString csText;
    ....
    if (m_nWindow > 0)
    csText.Format(_T("%s: %d"), csText, m_nWindow);
    ....
    }


    V547 Expression 'parentPos >= 0' is always true. Unsigned type value is always >= 0. bomgenerator.cpp 77

    void CBOMGenerator::GenerateBomForDesign(
    int level, size_t parentPos, ....)
    {
    ....
    if (parentPos >= 0) cmrParent = cmrDefaultParent;
    ....
    }


    V576 Incorrect format. A different number of actual arguments is expected while calling 'Format' function. Expected: 2. Present: 3. option.cpp 266
    V576 Incorrect format. A different number of actual arguments is expected while calling 'Format' function. Expected: 2. Present: 3. option.cpp 270

    CString COption::PointToDisplay(CDPoint a, BOOL horiz)
    {
    ....
    if (horiz)
    {
    r.Format(_T("%9.03f"), sy,
    GetUnits() == 0 ? _T("mm") : _T("\""));
    }
    else
    {
    r.Format(_T("%9.03f"), sx,
    GetUnits() == 0 ? _T("mm") : _T("\""));
    }
    ....
    }


    V595 The 'i' pointer was utilized before it was verified against nullptr. Check lines: 142, 144. drawmetafile.cpp 142

    bool CDrawMetaFile::setImageFile(const TCHAR filename)
    {
    CImage
    i = NULL;

    ....

    // Read in the buffer
    i->SetCompressedData(buffer, bytes);

    if (i != NULL)
    {
    m_metafile = m_pDesign->GetOptions()->AddMetaFile(i);
    }

    return i != NULL;
    }


    V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use'delete [] Np;'. context.cpp 446

    void CContext::PolyBezier(CDPoint pts, int Size)
    {
    // Make a new array to hold the points
    POINT
    Np = new POINT[Size];
    ....
    delete Np;
    }

    And:
    V611 The memory was allocated using 'new T[]' operator but was released using the'delete' operator. Consider inspecting this code. It's probably better to use 'delete[] pLogPal;'. imagepng.cpp 213
    V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use'delete [] buf;'. streamdb.cpp 182


    V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 86, 90. paint.cpp 86

    void CTinyCadView::OnDraw(CDC* pDC)
    {
    ....
    // Is any of this region in the off-page area?
    if (!pDC->IsPrinting())
    {
    // Paint the region white
    if (pDC->IsPrinting())
    {
    dc.SelectBrush(cWHITE);
    dc.SelectPen(PS_SOLID, 1, cWHITE);
    }
    ....
    }


    V636 The 'iwidth / actual_width[i]' expression was implicitly casted from'int' type to'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. context.cpp 712

    void CContext::TextOut(....)
    {
    int *actual_width = new int[len];
    double max_adjust = max_char_adjust;
    int iwidth;
    ....
    if (iwidth / actual_width[i] < 1.0 - max_adjust)
    ....
    }

    And:
    V636 The 'iwidth / actual_width[i]' expression was implicitly casted from'int' type to'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. context.cpp 717
    V636 The '3 / 2' expression was implicitly casted from'int' type to'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. drawjunction.cpp 179
    V636 The '3 / 2' expression was implicitly casted from'int' type to'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. drawjunction.cpp 185
    V636 The '5 / 2' expression was implicitly casted from'int' type to'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. drawmethod.cpp 1284
    V636 The '5 / 2' expression was implicitly casted from'int' type to'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. drawmethod.cpp 1301
    V636 The '5 / 2' expression was implicitly casted from'int' type to'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. editdlghierarchicaledit.cpp 513
    V636 The '5 / 2' expression was implicitly casted from'int' type to'double' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. editdlgmethodedit.cpp 561


    V668 There is no sense in testing the 'SortArray' pointer against null, as the memory was allocated using the'new' operator. The exception will be generated in the case of memory allocation error. imagepng.cpp 935

    bool CImagePNG::SubdivColorMap(....)
    {
    ....
    if ((SortArray = new QuantizedColorType*[....]) == NULL)
    return ERROR;
    ....
    }

    And:
    V668 There is no sense in testing the 'lpBMIH' pointer against null, as the memory was allocated using the'new' operator. The exception will be generated in the case of memory allocation error. imagepng.cpp 162


    Andrey Karpov.


    Sent from sourceforge.net because you indicated interest in
    https://sourceforge.net/p/tinycad/bugs/182/

    To unsubscribe from further messages, please visit
    https://sourceforge.net/auth/subscriptions/

     

    Related

    Bugs: #182


Log in to post a comment.