kicad-developers team mailing list archive

[jp charras <jp.charras@xxxxxxxxxx>]

Among issues detected by Coverity, 3 of them worried me.
(see attached text for these 3 Coverity bugs or strange things)

- In shape_collisions.cpp , CollideShapes compares 4 shapes.
One case ( SH_RECT / SH_RECT ) is strange, because the actual compaison
is SH_CIRCLE / SH_RECT.

Is it OK (if yes, a comment should be added), or a bug ?

- shape_poly_set.cpp has a bug for me.
Unless I missed something, a member of a deleted data (.m_next) is used
in the for loop tests.

- dialog_rescue_each.cpp
uses a dangerous code: pointers to explore a std::vector with its size
modified.
Usually it creates crashes because the pointers can be not valid when
the size of a std::vector changes.

Thanks to authors of this code to have a look at these issues.

-- 
Jean-Pierre CHARRAS

In /common/geometry/shape_collisions.cpp
diag: Using invalid iterator

251bool CollideShapes( const SHAPE* aA, const SHAPE* aB, int aClearance, bool aNeedMTV, VECTOR2I& aMTV )
252{
253    switch( aA->Type() )
254    {
   CID 102399 (#1 of 1): Missing break in switch (MISSING_BREAK)unterminated_case: The case for value SH_RECT is not terminated by a 'break' statement.
255        case SH_RECT:
256            switch( aB->Type() )
257            {
258                case SH_CIRCLE:
259                    return CollCase<SHAPE_RECT, SHAPE_CIRCLE>( aA, aB, aClearance, aNeedMTV, aMTV );
260
261                case SH_LINE_CHAIN:
262                    return CollCase<SHAPE_RECT, SHAPE_LINE_CHAIN>( aA, aB, aClearance, aNeedMTV, aMTV );
263
264                case SH_SEGMENT:
265                    return CollCase<SHAPE_RECT, SHAPE_SEGMENT>( aA, aB, aClearance, aNeedMTV, aMTV );
266
267                default:
268                    break;
269            }
270
Coverity diag: >>>>   fallthrough: The above case falls through to this one.
271        case SH_CIRCLE:
272            switch( aB->Type() )
273            {
274                case SH_RECT:
275                    return CollCaseReversed<SHAPE_CIRCLE, SHAPE_RECT>( aA, aB, aClearance, aNeedMTV, aMTV );

In /common/geometry/shape_poly_set.cpp
Diag: use data after deletion:

449    newPath.push_back(prev);
450    FractureEdge *e;
451    IntPoint p;
452
   CID 112252: Read from pointer after free (USE_AFTER_FREE) [select issue]
453    for( e = root; e->m_next != root; e=e->m_next)
454    {
455        p = e->m_p1;
456        newPath.push_back(p);
457        prev = p;
458        delete e;
459    }
460


File eeschema/dialogs/dialog_rescue_each.cpp:
Diag: Using invalid iterator

267bool DIALOG_RESCUE_EACH::TransferDataFromWindow()
268{
   1. Condition !this->wxWindowBase::TransferDataFromWindow(), taking false branch
269    if( !wxDialog::TransferDataFromWindow() )
270        return false;
271
272    std::vector<RESCUE_CANDIDATE>::iterator it = m_Candidates->begin();
   2. Condition it != this->m_Candidates->end(), taking true branch
   CID 109779 (#1 of 1): Using invalid iterator (INVALIDATE_ITERATOR)7. use_iterator: Using invalid iterator it.
273    for( size_t index = 0; it != m_Candidates->end(); ++index )
274    {
275        wxVariant val;
276        m_ListOfConflicts->GetValue( val, index, 0 );
277        bool rescue_part = val.GetBool();
278
   3. Condition !rescue_part, taking true branch
279        if( !rescue_part )
   4. erase_iterator: erase invalidates iterator it.
   5. Falling through to end of if statement
280            m_Candidates->erase( it );
281        else
282            ++it;
   6. Jumping back to the beginning of the loop
283    }
284
285    return true;
286}
287