kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #18743
Re: Questionable code
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
Follow ups
References