kicad-developers team mailing list archive

[Tomasz Wlostowski <tomasz.wlostowski@xxxxxxx>]

On 19.06.2015 18:35, jp charras wrote:
> 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 ?

A bug, but Rect/Rect test never takes place in the P&S (yet). To be
fixed, but there's no urgency.

> 
> - 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.

It's a bug, I've attached a patch. Good catch!.

Cheers,
Tom

>From fdeeafb5244d37eb047d4ba5a73f5f8712cc18b8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomasz=20W=C5=82ostowski?= <tomasz.wlostowski@xxxxxxx>
Date: Fri, 19 Jun 2015 18:49:28 +0200
Subject: [PATCH] SHAPE_POLY_SET: fix dereference of a deleted pointer

---
 common/geometry/shape_poly_set.cpp | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/common/geometry/shape_poly_set.cpp b/common/geometry/shape_poly_set.cpp
index b6bad11..a117f60 100644
--- a/common/geometry/shape_poly_set.cpp
+++ b/common/geometry/shape_poly_set.cpp
@@ -447,16 +447,19 @@ void SHAPE_POLY_SET::fractureSingle( ClipperLib::Paths& paths )
     paths.clear();
     Path newPath;
     newPath.push_back(prev);
-    FractureEdge *e;
+    FractureEdge *e, *e_next;
     IntPoint p;
 
-    for( e = root; e->m_next != root; e=e->m_next)
-    {
+    e = root;
+
+    do {
         p = e->m_p1;
         newPath.push_back(p);
         prev = p;
+        e_next = e->m_next;
         delete e;
-    }
+        e = e_next;
+    } while(e->m_next != root);
 
     p = e->m_p1;
 
-- 
1.9.1