To get a trial key
fill out the form below
Team License (standard version)
Enterprise License (extended version)
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Request our prices
New License
License Renewal
--Select currency--
USD
EUR
GBP
RUB
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
To get the licence for your open-source project, please fill out this form
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
To get the licence for your open-source project, please fill out this form
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
I am interested to try it on the platforms:
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Message submitted.

Your message has been sent. We will email you at


If you haven't received our response, please do the following:
check your Spam/Junk folder and click the "Not Spam" button for our message.
This way, you won't miss messages from our team in the future.

>
>
>
Following in the Footsteps of Calculato…

Following in the Footsteps of Calculators: SpeedCrunch

Mar 19 2019

Here we are, continuing to explore the code of calculators! Today we are going to take a look at the project called SpeedCrunch, the second most popular free calculator.

0618_SpeedCrunch/image1.png

Introduction

SpeedCrunch is a high-precision scientific calculator featuring a fast, keyboard-driven user interface. It is free and open-source software, licensed under the GPL and running on Windows, Linux, and macOS.

The source code is available on BitBucket. I was somewhat disappointed by the build documentation, which could be more detailed. It says that you need "Qt 5.2 or later" to build the project, but it actually required a few specific packages, which wasn't easy to figure out from the CMake log. By the way, it is considered a good practice nowadays to include a Dockerfile into the project to make it easier for the user to set up the development environment.

Here's output from the Cloc utility showing how SpeedCrunch compares to other calculators:

0618_SpeedCrunch/image2.png

Bug reviews for the other projects:

The analysis was done with the PVS-Studio static analyzer. This is a package of solutions for software quality control and detection of bugs and potential vulnerabilities. PVS-Studio supports C, C++, C#, and Java and runs on Windows, Linux, and macOS.

Strange logic in a loop

V560 A part of conditional expression is always true: !ruleFound. evaluator.cpp 1410

void Evaluator::compile(const Tokens& tokens)
{
  ....
  while (!syntaxStack.hasError()) {
    bool ruleFound = false;                                     // <=

    // Rule for function last argument: id (arg) -> arg.
    if (!ruleFound && syntaxStack.itemCount() >= 4) {           // <=
        Token par2 = syntaxStack.top();
        Token arg = syntaxStack.top(1);
        Token par1 = syntaxStack.top(2);
        Token id = syntaxStack.top(3);
        if (par2.asOperator() == Token::AssociationEnd
            && arg.isOperand()
            && par1.asOperator() == Token::AssociationStart
            && id.isIdentifier())
        {
            ruleFound = true;                                   // <=
            syntaxStack.reduce(4, MAX_PRECEDENCE);
            m_codes.append(Opcode(Opcode::Function, argCount));
#ifdef EVALUATOR_DEBUG
                dbg << "\tRule for function last argument "
                    << argCount << " \n";
#endif
            argCount = argStack.empty() ? 0 : argStack.pop();
        }
    }
    ....
  }
  ....
}

Note the ruleFound variable: it is set to false at each iteration. Inside the loop's body, though, that variable is set to true on certain conditions, but it will be set back to false at the next iteration. The ruleFound variable should have been probably declared before the loop.

Suspicious comparisons

V560 A part of conditional expression is always true: m_scrollDirection != 0. resultdisplay.cpp 242

void ResultDisplay::fullContentScrollEvent()
{
  QScrollBar* bar = verticalScrollBar();
  int value = bar->value();
  bool shouldStop = (m_scrollDirection == -1 && value <= 0) ||
                    (m_scrollDirection == 1 && value >= bar->maximum());

  if (shouldStop && m_scrollDirection != 0) {     // <=
      stopActiveScrollingAnimation();
      return;
  }

  scrollLines(m_scrollDirection * 10);
}

If the shouldStop variable's value is true, then the m_scrollDirection variable will take one of the two values: -1 or 1. Therefore, its value will definitely be different from zero in the next conditional statement, which is what the analyzer is warning about.

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

void EditorCompletion::showCompletion(const QStringList& choices)
{
  ....
  for (int i = 0; i < choices.count(); ++i) {
    QStringList pair = choices.at(i).split(':');
    QTreeWidgetItem* item = new QTreeWidgetItem(m_popup, pair);

    if (item && m_editor->layoutDirection() == Qt::RightToLeft)
        item->setTextAlignment(0, Qt::AlignRight);
    ....
  }
  ....
}

The memory for an object of type QTreeWidgetItem is allocated using the new operator. It means that a memory allocation failure will lead to throwing an std::bad_alloc() exception. Checking the item pointer is, therefore, redundant and can be removed.

Potential NULL Dereference

V595 The 'ioparams' pointer was utilized before it was verified against nullptr. Check lines: 969, 983. floatio.c 969

int cattokens(....)
{
  ....
  if (printexp)
  {
    if (expbase < 2)
      expbase = ioparams->expbase;  // <=
    ....
  }
  dot = '.';
  expbegin = "(";
  expend = ")";
  if (ioparams != NULL)            // <=
  {
    dot = ioparams->dot;
    expbegin = ioparams->expbegin;
    expend = ioparams->expend;
  }
  ....
}

The ioparams pointer is dereferenced before the check. It looks like there's some mistake here. Since the dereference is preceded by a number of conditions, the bug won't show up often, but it'll have a drastic effect when it does.

Division by zero

V609 Divide by zero. Denominator range [0..4]. floatconvert.c 266

static int
lgbase( signed char base)
{
  switch(base)
  {
    case 2:
      return 1;
    case 8:
      return 3;
    case 16:
      return 4;
  }
  return 0;                                       // <=
}

static void
_setlongintdesc(
  p_ext_seq_desc n,
  t_longint* l,
  signed char base)
{
  int lg;

  n->seq.base = base;
  lg = lgbase(base);                              // <=
  n->seq.digits = (_bitlength(l) + lg - 1) / lg;  // <=
  n->seq.leadingSignDigits = 0;
  n->seq.trailing0 = _lastnonzerobit(l) / lg;     // <=
  n->seq.param = l;
  n->getdigit = _getlongintdigit;
}

The lgbase function can return zero, which could then be used as a divisor. The function can be potentially called with any value, not only 2, 8, or 16.

Undefined behavior

V610 Undefined behavior. Check the shift operator '<<'. The left operand '(~0)' is negative. floatlogic.c 64

static char
_signextend(
  t_longint* longint)
{
  unsigned mask;
  signed char sign;

  sign = _signof(longint);
  mask = (~0) << SIGNBIT;  // <=
  if (sign < 0)
    longint->value[MAXIDX] |= mask;
  else
    longint->value[MAXIDX] &= ~mask;
  return sign;
}

Because the result of inverting zero is stored to a signed int, the resulting value will be a negative number, which is then shifted. Left-shifting a negative value is undefined behavior.

Here's a complete list of all such cases:

  • V610 Undefined behavior. Check the shift operator '<<'. The left operand '(- 1)' is negative. floatnum.c 289
  • V610 Undefined behavior. Check the shift operator '<<'. The left operand '(- 1)' is negative. floatnum.c 325
  • V610 Undefined behavior. Check the shift operator '<<'. The left operand '(- 1)' is negative. floatnum.c 344
  • V610 Undefined behavior. Check the shift operator '<<'. The left operand '(- 1)' is negative. floatnum.c 351

Unclosed HTML tags

V735 Possibly an incorrect HTML. The "</body>" closing tag was encountered, while the "</div>" tag was expected. book.cpp 127

static QString makeAlgebraLogBaseConversionPage() {
  return
    BEGIN
    INDEX_LINK
    TITLE(Book::tr("Logarithmic Base Conversion"))
    FORMULA(y = log(x) / log(a), log<sub>a</sub>x = log(x) / log(a))
    END;
}

As is often the case with C/C++ code, studying the source doesn't help much in figuring things out, so we'll take a look at the preprocessed code instead:

0618_SpeedCrunch/image3.png

The analyzer has detected an unclosed div tag. This file contains lots of snippets in HTML, and the developers will have to check that code too.

Here are a couple of other suspicious cases found by PVS-Studio:

  • V735 Possibly an incorrect HTML. The "</td>" closing tag was encountered, while the "</sub>" tag was expected. book.cpp 344
  • V735 Possibly an incorrect HTML. The "</td>" closing tag was encountered, while the "</sub>" tag was expected. book.cpp 347

Assignment operator

V794 The assignment operator should be protected from the case of 'this == &other'. quantity.cpp 373

Quantity& Quantity::operator=(const Quantity& other)
{
  m_numericValue = other.m_numericValue;
  m_dimension = other.m_dimension;
  m_format = other.m_format;
  stripUnits();
  if(other.hasUnit()) {
    m_unit = new CNumber(*other.m_unit);
    m_unitName = other.m_unitName;
  }
  cleanDimension();
  return *this;
}

It is recommended that you check situations where an object is assigned to itself by comparing the pointers. In other words, add the following two lines to the beginning of the function body:

if (this == &other)
  return *this;

As a reminder

V601 The 'false' value is implicitly cast to the integer type. cmath.cpp 318

/**
 * Returns -1, 0, 1 if n1 is less than, equal to, or more than n2.
 * Only valid for real numbers, since complex ones are not an ordered field.
 */
int CNumber::compare(const CNumber& other) const
{
  if (isReal() && other.isReal())
    return real.compare(other.real);
  else
    return false; // FIXME: Return something better.
}

Sometimes you say in comments that maybe some of the warnings are triggered by incomplete code. Yes, that happens every now and then, but we specifically point such cases out.

Conclusion

We have already checked the code of three calculator programs - Windows Calculator, Qalculate!, and SpeedCrunch - and aren't going to stop. Feel free to suggest projects you want us to check because software rankings don't always reflect the real state of things.

Welcome to download PVS-Studio and try it on your own "Calculator". :-)

Popular related articles
Characteristics of PVS-Studio Analyzer by the Example of EFL Core Libraries, 10-15% of False Positives

Date: Jul 31 2017

Author: Andrey Karpov

After I wrote quite a big article about the analysis of the Tizen OS code, I received a large number of questions concerning the percentage of false positives and the density of errors (how many erro…
Free PVS-Studio for those who develops open source projects

Date: Dec 22 2018

Author: Andrey Karpov

On the New 2019 year's eve, a PVS-Studio team decided to make a nice gift for all contributors of open-source projects hosted on GitHub, GitLab or Bitbucket. They are given free usage of PVS-Studio s…
PVS-Studio ROI

Date: Jan 30 2019

Author: Andrey Karpov

Occasionally, we're asked a question, what monetary value the company will receive from using PVS-Studio. We decided to draw up a response in the form of an article and provide tables, which will sho…
Appreciate Static Code Analysis!

Date: Oct 16 2017

Author: Andrey Karpov

I am really astonished by the capabilities of static code analysis even though I am one of the developers of PVS-Studio analyzer myself. The tool surprised me the other day as it turned out to be sma…
The way static analyzers fight against false positives, and why they do it

Date: Mar 20 2017

Author: Andrey Karpov

In my previous article I wrote that I don't like the approach of evaluating the efficiency of static analyzers with the help of synthetic tests. In that article, I give the example of a code fragment…
Static analysis as part of the development process in Unreal Engine

Date: Jun 27 2017

Author: Andrey Karpov

Unreal Engine continues to develop as new code is added and previously written code is changed. What is the inevitable consequence of ongoing development in a project? The emergence of new bugs in th…
Technologies used in the PVS-Studio code analyzer for finding bugs and potential vulnerabilities

Date: Nov 21 2018

Author: Andrey Karpov

A brief description of technologies used in the PVS-Studio tool, which let us effectively detect a large number of error patterns and potential vulnerabilities. The article describes the implementati…
How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers

Date: Oct 22 2018

Author: Andrey Karpov

Just like other static analyzers, PVS-Studio often produces false positives. What you are about to read is a short story where I'll tell you how PVS-Studio proved, just one more time, to be more atte…
The Evil within the Comparison Functions

Date: May 19 2017

Author: Andrey Karpov

Perhaps, readers remember my article titled "Last line effect". It describes a pattern I've once noticed: in most cases programmers make an error in the last line of similar text blocks. Now I want t…
The Last Line Effect

Date: May 31 2014

Author: Andrey Karpov

I have studied many errors caused by the use of the Copy-Paste method, and can assure you that programmers most often tend to make mistakes in the last fragment of a homogeneous code block. I have ne…

Comments (0)

Next comments

This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
This website uses cookies and other technology to provide you a more personalized experience. By continuing the view of our web-pages you accept the terms of using these files. If you don't want your personal data to be processed, please, leave this site.
Learn More →
Accept