Project

General

Profile

Bug #1020

Cppcheck: Missing bounds check for extra iterator increment in loop.

Added by Thomas Beutlich over 4 years ago. Updated over 4 years ago.

Status:
Rejected
Priority:
Normal
Assignee:
Category:
samples
Target version:
Start date:
05 Jan 2015
Due date:
% Done:

0%

Estimated time:

Description

Cppcheck reports for Jzon.cpp line 469

The iterator incrementing is suspicious - it is incremented at line 469 and then at line 457. The loop might unintentionally skip an element in the container. There is no comparison between these increments to prevent that the iterator is incremented beyond the end.

History

#1

Updated by Robin Mills over 4 years ago

  • Category set to coverity
  • Status changed from New to Assigned
  • Assignee set to Robin Mills
  • Target version set to 0.25
#2

Updated by Robin Mills over 4 years ago

  • Category changed from coverity to samples
  • Status changed from Assigned to Resolved

I think the code is good! Here's the code:

    std::string Value::UnescapeString(const std::string &value)
    {
        std::string unescaped;

457:    for (std::string::const_iterator it = value.begin(); it != value.end(); ++it)
        {
            const char &c = (*it);
            char c2 = '\0';
            if (it+1 != value.end())
                c2 = *(it+1);

            const char &a = getUnescaped(c, c2);
            if (a != '\0')
            {
                unescaped += a;
                if (it+1 != value.end())
469:                    ++it;
            }
            else
            {
                unescaped += c;
            }
        }

        return unescaped;
    }
When it encounters an escape character, "it" is incremented. The condition "if ( it+1 != value.end() )" prevents the loop from overstepping.

It may be better to write this code as a while loop to keep Cppcheck quiet. However the Jzon code was imported from an external source and I won't change it without good cause. In fact, it would be good to consider upgrading to the latest Jzon offering, however there is no reason to change anything at this time.

If you are happy with my explanation, then I'd like to change the status of this issue to "Rejected" as this will save time during the issue review process for v0.25.

#3

Updated by Thomas Beutlich over 4 years ago

Yep, it's fine.

#4

Updated by Robin Mills over 4 years ago

  • Status changed from Resolved to Rejected

Also available in: Atom PDF