kicad-developers team mailing list archive

[John Beard <john.j.beard@xxxxxxxxx>]

Hi,

Small patch here to fix a small leak in a unit test.

While the leak is not very exciting here, the other function of unit
tests is to "document" interfaces by using them correctly. So this
uses a unique_ptr to make the ownership explicit.

Fixes coverity defect 183869.

Cheers,

John

From 53e1c92fbdf204d7070258cf27b30e156ddc37e5 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Fri, 28 Sep 2018 17:59:42 +0100
Subject: [PATCH] QA: Fix ownership of new CPolyline in
 qa_shape_poly_set_refactor

CPolyLine::Chamfer returns a newly allocated object. The current
test of this doesn't take ownership, leading to memory leaks.

Use a std::unique_ptr to:

* Fix the leak, and
* Document the expected semantics of the interface in the test

Fixes coverity:183869
---
 qa/shape_poly_set_refactor/test_chamfer.cpp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/qa/shape_poly_set_refactor/test_chamfer.cpp b/qa/shape_poly_set_refactor/test_chamfer.cpp
index 22dc245c4..7d8a16859 100644
--- a/qa/shape_poly_set_refactor/test_chamfer.cpp
+++ b/qa/shape_poly_set_refactor/test_chamfer.cpp
@@ -99,23 +99,24 @@ void TestLineChainEqualCPolyLine(SHAPE_LINE_CHAIN& lineChain, CPolyLine& polyLin
 BOOST_AUTO_TEST_CASE( Chamfer )
 {
     SHAPE_POLY_SET::POLYGON actual;
-    CPolyLine expected;
 
     // Test different distances, up to the half of the minimum segment longitude
-    for (int distance = 0; distance < 5; distance++) {
+    for( int distance = 0; distance < 5; distance++ )
+    {
         // Chamfered polygon to be tested.
         actual = common.holeyPolySet.ChamferPolygon( distance, 0 );
 
         // Chamfered polygon assumed to be right.
-        expected = *legacyPolyLine.Chamfer( distance );
+        // CPolyline::Chamfer returns new CPolyline - take ownership
+        std::unique_ptr<CPolyLine> expected( legacyPolyLine.Chamfer( distance ) );
 
         // Double check that there are no repeated corners in the legacy shape.
-        expected.RemoveNullSegments();
+        expected->RemoveNullSegments();
 
         // Test equality
-        for (size_t contourIdx = 0; contourIdx < actual.size(); contourIdx++)
+        for( size_t contourIdx = 0; contourIdx < actual.size(); contourIdx++ )
         {
-            TestLineChainEqualCPolyLine(actual[contourIdx], expected, contourIdx);
+            TestLineChainEqualCPolyLine(actual[contourIdx], *expected, contourIdx);
         }
     }
 }
-- 
2.19.0