From 1a5df45da079f07c5b3a8bf3b6071c005d7fffca Mon Sep 17 00:00:00 2001
From: Yann Dirson <ydirson@altern.org>
Date: Sun, 20 Apr 2008 21:52:43 +0200
Subject: [PATCH] Address some valgrind-detected problems, flag others for
 later.

---
 src/chessboard.cpp        |  8 ++++++--
 src/chesstable.cpp        |  2 +-
 src/clock.cpp             |  3 ++-
 src/core/board.cpp        |  2 +-
 src/core/defaultstate.cpp |  4 +++-
 src/core/defaulttype.cpp  | 17 +++++++++--------
 src/core/repository.cpp   |  6 +++++-
 7 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/src/chessboard.cpp b/src/chessboard.cpp
index 1cbd185..2c90053 100644
--- a/src/chessboard.cpp
+++ b/src/chessboard.cpp
@@ -74,6 +74,10 @@ ChessBoard::~ChessBoard() {
   while(!m_canvas_border->items()->isEmpty())
     delete m_canvas_border->items()->first();
   delete m_canvas_border;
+
+  while(!m_canvas_border_text->items()->isEmpty())
+    delete m_canvas_border_text->items()->first();
+  delete m_canvas_border_text;
 }
 
 void ChessBoard::settingsChanged() {
@@ -173,7 +177,7 @@ void ChessBoard::recreateBorder() {
   for(int i = 0;i<s.x;i++) {
     int c = w ? i : i+s.x+s.y;
     QString l = m_border_coords.size()>c ? m_border_coords[c] : QString();
-    ConstrainedText *item = new ConstrainedText(m_canvas_border_text);
+    ConstrainedText *item = new ConstrainedText(m_canvas_border_text); // FIXME[vg]: leaked
     item->setColor(m_border_text_color);
     item->setText(l);
     item->setFont(m_border_font);
@@ -186,7 +190,7 @@ void ChessBoard::recreateBorder() {
   for(int i = 0;i<s.y;i++) {
     int c = w ? i+s.x : i+2*s.x+s.y;
     QString n = m_border_coords.size()>c ? m_border_coords[c] : QString();
-    ConstrainedText *item = new ConstrainedText(m_canvas_border_text);
+    ConstrainedText *item = new ConstrainedText(m_canvas_border_text); // FIXME[vg]: leaked
     item->setColor(m_border_text_color);
     item->setText(n);
     item->setFont(m_border_font);
diff --git a/src/chesstable.cpp b/src/chesstable.cpp
index 468cce3..c8620dd 100644
--- a/src/chesstable.cpp
+++ b/src/chesstable.cpp
@@ -106,7 +106,7 @@ void ChessTable::renderWallpaper() {
     }
     else {
       delete m_wallpaper;
-      m_wallpaper = new KGameCanvasPixmap(QPixmap::fromImage(res), this);
+      m_wallpaper = new KGameCanvasPixmap(QPixmap::fromImage(res), this); // FIXME[vg]: leaked
       m_wallpaper->lower();
       m_wallpaper->show();
     }
diff --git a/src/clock.cpp b/src/clock.cpp
index 1a9e6e5..ef5cf8f 100644
--- a/src/clock.cpp
+++ b/src/clock.cpp
@@ -16,7 +16,8 @@
 Clock::Clock(int col, KGameCanvasAbstract* canvas)
 : ClickableCanvas(canvas)
 , m_color(col)
-, m_running(false) {
+, m_running(false)
+, m_active(false) {
   m_background   = new KGameCanvasPixmap(this);
   m_caption      = new ConstrainedText(this);
   m_time_label   = new ConstrainedText(this);
diff --git a/src/core/board.cpp b/src/core/board.cpp
index 64366da..d3ea9a3 100644
--- a/src/core/board.cpp
+++ b/src/core/board.cpp
@@ -48,7 +48,7 @@ Piece Board::get(const Point& p) const {
 
 void Board::set(const Point& p, const Piece& piece) {
   if (valid(p)) {
-    m_data[p.x + p.y * m_size.x] = piece;
+    m_data[p.x + p.y * m_size.x] = piece;	  // FIXME[vg]: leaks former Piece ?
   }
   else
     kError() << "point is not valid" << p;
diff --git a/src/core/defaultstate.cpp b/src/core/defaultstate.cpp
index 7df4508..357f78a 100644
--- a/src/core/defaultstate.cpp
+++ b/src/core/defaultstate.cpp
@@ -23,9 +23,11 @@ std::vector<const Point*> DefaultState::theoreticalMoves(const Piece& piece,
 					    -behaviour->direction(piece.color()).y),
 				   src.y + ((*defs)[direction].y * distance *
 					    behaviour->direction(piece.color()).y));
-      if (!board()->valid(*candidate))
+      if (!board()->valid(*candidate)) {
 	// end of board: don't look further in that direction
+	delete candidate;
 	break;
+      }
 
       destinations.push_back(candidate);
 
diff --git a/src/core/defaulttype.cpp b/src/core/defaulttype.cpp
index 889b4f2..11fc235 100644
--- a/src/core/defaulttype.cpp
+++ b/src/core/defaulttype.cpp
@@ -11,17 +11,18 @@ bool DefaultType::canMove(const Piece& piece, const Piece&,
     kDebug() << "use of a non-DefaultState-derived state with DefaultType";
     return false;
   }
+
   std::vector<const Point*> moves = dstate->theoreticalMoves(piece, move.src());
-  std::vector<const Point*>::const_iterator it;
-  for (it = moves.begin(); it != moves.end(); it++) {
+
+  // theoreticalMoves() allocates Point objects for us, we must ensure
+  // all of them are freed, so we visit the whole moves vector.
+  bool ret = false;
+  for (std::vector<const Point*>::const_iterator it = moves.begin();
+       it != moves.end(); it++) {
     if (move.dst() == **it)
-      return true;
+      ret = true;
     delete *it;
   }
 
-  // delete remaining points in the vector
-  for (; it != moves.end(); it++)
-    delete *it;
-
-  return false;
+  return ret;
 }
diff --git a/src/core/repository.cpp b/src/core/repository.cpp
index 83a270d..88b6096 100644
--- a/src/core/repository.cpp
+++ b/src/core/repository.cpp
@@ -14,7 +14,11 @@ Repository::Repository()
 : m_proxy(0) { }
 
 void Repository::addComponent(const QString& path, ComponentPtr component) {
-  m_components[path] = component;
+  if (m_components.contains(path))
+    kError() << "NOT replacing component" << path <<
+      "in component" << component->objectName();
+  else
+    m_components[path] = component;
 }
 
 ComponentPtr Repository::getComponent(const QString& path) const {
-- 
2.11.4.GIT