Screen and HistoryScroll pointers to smart pointer

Those Classes are not QObject, so most of it's pointers could be changed
 to smart pointers with no problems.
Problem found when using QList<std::unique_ptr>: it wasn't possible to
use it. I'm using std containers instead.

src/Screen.cpp

@@ -60,7 +60,7 @@ Screen::Screen(int lines, int columns):
     _isResize(false),
     _enableReflowLines(false),
     _lineProperties(QVarLengthArray<LineProperty, 64>()),
-    _history(new HistoryScrollNone()),
+    _history(std::make_unique<HistoryScrollNone>()),
     _cuX(0),
     _cuY(0),
     _currentForeground(CharacterColor()),
@@ -78,7 +78,7 @@ Screen::Screen(int lines, int columns):
     _effectiveRendition(DEFAULT_RENDITION),
     _lastPos(-1),
     _lastDrawnChar(0),
-    _escapeSequenceUrlExtractor(new EscapeSequenceUrlExtractor())
+    _escapeSequenceUrlExtractor(std::make_unique<EscapeSequenceUrlExtractor>())
 {
     _escapeSequenceUrlExtractor->setScreen(this);
     _lineProperties.resize(_lines + 1);
@@ -91,8 +91,6 @@ Screen::Screen(int lines, int columns):
 
 Screen::~Screen()
 {
-    delete _history;
-    delete _escapeSequenceUrlExtractor;
 }
 
 void Screen::cursorUp(int n)
@@ -885,12 +883,11 @@ void Screen::displayCharacter(uint c)
             if (((oldChars) != nullptr) && extendedCharLength < 3) {
                 Q_ASSERT(extendedCharLength > 1);
                 Q_ASSERT(extendedCharLength < 65535); // redundant due to above check
-                auto chars = new uint[extendedCharLength + 1];
-                memcpy(chars, oldChars, sizeof(uint) * extendedCharLength);
+                auto chars = std::make_unique<uint[]>(extendedCharLength + 1);
+                std::copy_n(oldChars, extendedCharLength, chars.get());
                 chars[extendedCharLength] = c;
                 auto extChars = [this]() { return usedExtendedChars(); };
-                currentChar.character = ExtendedCharTable::instance.createExtendedChar(chars, extendedCharLength + 1, extChars);
-                delete[] chars;
+                currentChar.character = ExtendedCharTable::instance.createExtendedChar(chars.get(), extendedCharLength + 1, extChars);
             }
         }
         return;
@@ -1662,16 +1659,17 @@ int Screen::getHistLines() const
     return _history->getLines();
 }
 
-void Screen::setScroll(const HistoryType& t , bool copyPreviousScroll)
+void Screen::setScroll(const HistoryType &t, bool copyPreviousScroll)
 {
     clearSelection();
 
     if (copyPreviousScroll) {
-        _history = t.scroll(_history);
+        t.scroll(_history);
     } else {
-        HistoryScroll* oldScroll = _history;
-        _history = t.scroll(nullptr);
-        delete oldScroll;
+        // As 't' can be '_history' pointer, move it to a temporary smart pointer
+        // making _history = nullptr
+        auto oldHistory = std::move(_history);
+        t.scroll(_history);
     }
 }
 
@@ -1680,7 +1678,7 @@ bool Screen::hasScroll() const
     return _history->hasScroll();
 }
 
-const HistoryType& Screen::getScroll() const
+const HistoryType &Screen::getScroll() const
 {
     return _history->getType();
 }
@@ -1700,5 +1698,5 @@ void Screen::fillWithDefaultChar(Character* dest, int count)
 
 Konsole::EscapeSequenceUrlExtractor * Konsole::Screen::urlExtractor() const
 {
-    return _escapeSequenceUrlExtractor;
+    return _escapeSequenceUrlExtractor.get();
 }

src/Screen.h

@@ -8,6 +8,9 @@
 #ifndef SCREEN_H
 #define SCREEN_H
 
+// STD
+#include <memory>
+
 // Qt
 #include <QRect>
 #include <QSet>
@@ -683,7 +686,7 @@ private:
     QVarLengthArray<LineProperty, 64> _lineProperties;
 
     // history buffer ---------------
-    HistoryScroll *_history;
+    std::unique_ptr<HistoryScroll> _history;
 
     // cursor location
     int _cuX;
@@ -742,7 +745,7 @@ private:
 
     // used in REP (repeating char)
     quint32 _lastDrawnChar;
-    EscapeSequenceUrlExtractor *_escapeSequenceUrlExtractor;
+    std::unique_ptr<EscapeSequenceUrlExtractor> _escapeSequenceUrlExtractor;
     void toggleUrlInput();
 
     // Vt102Emulation defined max argument value that can be passed to a Screen function

src/autotests/HistoryTest.cpp

@@ -196,7 +196,7 @@ void HistoryTest::testHistoryReflow()
 
 void HistoryTest::testHistoryTypeChange()
 {
-    HistoryScroll *historyScroll = nullptr;
+    std::unique_ptr<HistoryScroll> historyScroll(nullptr);
 
     const char testString[] = "abcdefghijklmnopqrstuvwxyz1234567890";
     const int testStringSize = sizeof(testString) / sizeof(char) - 1;
@@ -208,11 +208,11 @@ void HistoryTest::testHistoryTypeChange()
 
     // None
     auto historyTypeNone = new HistoryTypeNone();
-    historyScroll = historyTypeNone->scroll(historyScroll);
+    historyTypeNone->scroll(historyScroll);
 
     // None to File
     auto historyTypeFile = new HistoryTypeFile();
-    historyScroll = historyTypeFile->scroll(historyScroll);
+    historyTypeFile->scroll(historyScroll);
 
     historyScroll->addCells(testImage, testStringSize);
     historyScroll->addLine(false);
@@ -223,23 +223,22 @@ void HistoryTest::testHistoryTypeChange()
 
     // File to Compact
     auto compactHistoryType = new CompactHistoryType(10);
-    historyScroll = compactHistoryType->scroll(historyScroll);
+    compactHistoryType->scroll(historyScroll);
 
     QCOMPARE(historyScroll->getLines(), 10);
     historyScroll->getCells(0, 0, 1, &testChar);
     QCOMPARE(testChar, testImage[testStringSize - 10]);
 
     // Compact to File
-    historyScroll = historyTypeFile->scroll(historyScroll);
+    historyTypeFile->scroll(historyScroll);
     QCOMPARE(historyScroll->getLines(), 10);
     historyScroll->getCells(0, 0, 1, &testChar);
     QCOMPARE(testChar, testImage[testStringSize - 10]);
 
     // File to None
-    historyScroll = historyTypeNone->scroll(historyScroll);
+    historyTypeNone->scroll(historyScroll);
     QCOMPARE(historyScroll->getLines(), 0);
 
-    delete historyScroll;
     delete historyTypeFile;
     delete historyTypeNone;
     delete compactHistoryType;

src/history/HistoryScroll.cpp

@@ -18,7 +18,6 @@ HistoryScroll::HistoryScroll(HistoryType *t) :
 
 HistoryScroll::~HistoryScroll()
 {
-    delete _historyType;
 }
 
 bool HistoryScroll::hasScroll()

src/history/HistoryScroll.h

@@ -7,6 +7,9 @@
 #ifndef HISTORYSCROLL_H
 #define HISTORYSCROLL_H
 
+// STD
+#include <memory>
+
 #include "konsoleprivate_export.h"
 
 // Konsole
@@ -68,7 +71,7 @@ public:
     }
 
 protected:
-    HistoryType *_historyType;
+    std::unique_ptr<HistoryType> _historyType;
     const int MAX_REFLOW_LINES = 20000;
 };
 

src/history/HistoryScrollFile.cpp

@@ -125,7 +125,7 @@ void HistoryScrollFile::setCellsVectorAt(int, const QVector<Character> &)
 
 int HistoryScrollFile::reflowLines(int columns)
 {
-    HistoryFile *reflowFile = new HistoryFile;
+    auto reflowFile = std::make_unique<HistoryFile>();
     reflowData newLine;
 
     auto reflowLineLen = [] (qint64 start, qint64 end) {
@@ -184,7 +184,6 @@ int HistoryScrollFile::reflowLines(int columns)
         currentPos++;
     }
 
-    delete reflowFile;
     return 0;
 }
 

src/history/HistoryType.h

@@ -33,7 +33,7 @@ public:
      * Converts from one type of HistoryScroll to another or if given the
      * same type, returns it.
      */
-    virtual HistoryScroll *scroll(HistoryScroll *) const = 0;
+    virtual void scroll(std::unique_ptr<HistoryScroll> &) const = 0;
     /**
      * Returns true if the history size is unlimited.
      */

src/history/HistoryTypeFile.cpp

@@ -20,23 +20,22 @@ bool HistoryTypeFile::isEnabled() const
     return true;
 }
 
-HistoryScroll *HistoryTypeFile::scroll(HistoryScroll *old) const
+void HistoryTypeFile::scroll(std::unique_ptr<HistoryScroll> &old) const
 {
-    if (dynamic_cast<HistoryFile *>(old) != nullptr) {
-        return old; // Unchanged.
+    if (dynamic_cast<HistoryFile *>(old.get()) != nullptr) {
+        return; // Unchanged.
     }
-    HistoryScroll *newScroll = new HistoryScrollFile();
+    auto newScroll = std::make_unique<HistoryScrollFile>();
 
     Character line[LINE_SIZE];
     int lines = (old != nullptr) ? old->getLines() : 0;
     for (int i = 0; i < lines; i++) {
         int size = old->getLineLen(i);
         if (size > LINE_SIZE) {
-            auto tmp_line = new Character[size];
-            old->getCells(i, 0, size, tmp_line);
-            newScroll->addCells(tmp_line, size);
+            auto tmp_line = std::make_unique<Character[]>(size);
+            old->getCells(i, 0, size, tmp_line.get());
+            newScroll->addCells(tmp_line.get(), size);
             newScroll->addLine(old->isWrappedLine(i));
-            delete [] tmp_line;
         } else {
             old->getCells(i, 0, size, line);
             newScroll->addCells(line, size);
@@ -44,8 +43,7 @@ HistoryScroll *HistoryTypeFile::scroll(HistoryScroll *old) const
         }
     }
 
-    delete old;
-    return newScroll;
+    old = std::move(newScroll);
 }
 
 int HistoryTypeFile::maximumLineCount() const

src/history/HistoryTypeFile.h

@@ -21,7 +21,7 @@ public:
     bool isEnabled() const override;
     int maximumLineCount() const override;
 
-    HistoryScroll *scroll(HistoryScroll *) const override;
+    void scroll(std::unique_ptr<HistoryScroll> &) const override;
 };
 
 }

src/history/HistoryTypeNone.cpp

@@ -16,10 +16,9 @@ bool HistoryTypeNone::isEnabled() const
     return false;
 }
 
-HistoryScroll *HistoryTypeNone::scroll(HistoryScroll *old) const
+void HistoryTypeNone::scroll(std::unique_ptr<HistoryScroll> &old) const
 {
-    delete old;
-    return new HistoryScrollNone();
+    old = std::make_unique<HistoryScrollNone>();
 }
 
 int HistoryTypeNone::maximumLineCount() const

src/history/HistoryTypeNone.h

@@ -21,7 +21,7 @@ public:
     bool isEnabled() const override;
     int maximumLineCount() const override;
 
-    HistoryScroll *scroll(HistoryScroll *) const override;
+    void scroll(std::unique_ptr<HistoryScroll> &) const override;
 };
 
 }

src/history/compact/CompactHistoryScroll.cpp

@@ -23,19 +23,17 @@ CompactHistoryScroll::CompactHistoryScroll(unsigned int maxLineCount) :
 
 CompactHistoryScroll::~CompactHistoryScroll()
 {
-    qDeleteAll(_lines.begin(), _lines.end());
     _lines.clear();
 }
 
 void CompactHistoryScroll::addCellsVector(const TextLine &cells)
 {
-    CompactHistoryLine *line;
-    line = new (_blockList) CompactHistoryLine(cells, _blockList);
+    auto line = std::unique_ptr<CompactHistoryLine>(new (_blockList) CompactHistoryLine(cells, _blockList));
 
-    _lines.append(line);
+    _lines.push_back(std::move(line));
 
-    if (_lines.size() > static_cast<int>(_maxLineCount)) {
-        delete _lines.takeAt(0);
+    if (_lines.size() > static_cast<size_t>(_maxLineCount)) {
+        _lines.pop_front();
     }
 }
 
@@ -48,7 +46,7 @@ void CompactHistoryScroll::addCells(const Character a[], int count)
 
 void CompactHistoryScroll::addLine(bool previousWrapped)
 {
-    CompactHistoryLine *line = _lines.last();
+    auto line = _lines.back().get();
     ////qDebug() << "last line at address " << line;
     line->setWrapped(previousWrapped);
 }
@@ -65,12 +63,12 @@ int CompactHistoryScroll::getMaxLines()
 
 int CompactHistoryScroll::getLineLen(int lineNumber)
 {
-    if ((lineNumber < 0) || (lineNumber >= _lines.size())) {
+    if ((lineNumber < 0) || ((size_t)lineNumber >= _lines.size())) {
         //qDebug() << "requested line invalid: 0 < " << lineNumber << " < " <<_lines.size();
         //Q_ASSERT(lineNumber >= 0 && lineNumber < _lines.size());
         return 0;
     }
-    CompactHistoryLine *line = _lines[lineNumber];
+    auto line = _lines[lineNumber].get();
     ////qDebug() << "request for line at address " << line;
     return line->getLength();
 }
@@ -81,7 +79,7 @@ void CompactHistoryScroll::getCells(int lineNumber, int startColumn, int count,
         return;
     }
     Q_ASSERT(lineNumber < _lines.size());
-    CompactHistoryLine *line = _lines[lineNumber];
+    auto line = _lines[lineNumber].get();
     Q_ASSERT(startColumn >= 0);
     Q_ASSERT(static_cast<unsigned int>(startColumn) <= line->getLength() - count);
     line->getCharacters(buffer, count, startColumn);
@@ -91,20 +89,20 @@ void CompactHistoryScroll::setMaxNbLines(unsigned int lineCount)
 {
     _maxLineCount = lineCount;
 
-    while (_lines.size() > static_cast<int>(lineCount)) {
-        delete _lines.takeAt(0);
+    while (_lines.size() > static_cast<size_t>(lineCount)) {
+        _lines.pop_front();
     }
     ////qDebug() << "set max lines to: " << _maxLineCount;
 }
 
 void CompactHistoryScroll::insertCellsVector(int position, const TextLine &cells)
 {
-    CompactHistoryLine *line = new (_blockList) CompactHistoryLine(cells, _blockList);
+    auto line = std::unique_ptr<CompactHistoryLine>(new (_blockList) CompactHistoryLine(cells, _blockList));
 
-    _lines.insert(position, line);
+    _lines.insert(_lines.begin() + position, std::move(line));
 
-    if (_lines.size() > static_cast<int>(_maxLineCount)) {
-        delete _lines.takeAt(0);
+    if (_lines.size() > static_cast<size_t>(_maxLineCount)) {
+        _lines.pop_front();
     }
 }
 
@@ -117,7 +115,7 @@ void CompactHistoryScroll::insertCells(int position, const Character a[], int co
 
 void CompactHistoryScroll::removeCells(int position)
 {
-    delete _lines.takeAt(position);
+    _lines.erase(_lines.begin() + position);
 }
 
 void CompactHistoryScroll::setCellsAt(int position, const Character a[], int count)
@@ -134,7 +132,7 @@ void CompactHistoryScroll::setCellsVectorAt(int position, const TextLine &cells)
 
 void CompactHistoryScroll::setLineAt(int position, bool previousWrapped)
 {
-    CompactHistoryLine *line = _lines.at(position);
+    auto line = _lines.at(position).get();
 
     line->setWrapped(previousWrapped);
 }

src/history/compact/CompactHistoryScroll.h

@@ -7,6 +7,9 @@
 #ifndef COMPACTHISTORYSCROLL_H
 #define COMPACTHISTORYSCROLL_H
 
+// STD
+#include <deque>
+
 #include "konsoleprivate_export.h"
 
 #include "history/HistoryScroll.h"
@@ -17,7 +20,7 @@ namespace Konsole
 
 class KONSOLEPRIVATE_EXPORT CompactHistoryScroll : public HistoryScroll
 {
-    typedef QList<CompactHistoryLine *> HistoryArray;
+    typedef std::deque<std::unique_ptr<CompactHistoryLine>> HistoryArray;
 
 public:
     explicit CompactHistoryScroll(unsigned int maxLineCount = 1000);

src/history/compact/CompactHistoryType.cpp

@@ -29,37 +29,29 @@ int CompactHistoryType::maximumLineCount() const
     return _maxLines;
 }
 
-HistoryScroll *CompactHistoryType::scroll(HistoryScroll *old) const
+void CompactHistoryType::scroll(std::unique_ptr<HistoryScroll> &old) const
 {
-    if (old != nullptr) {
-        auto *newBuffer = dynamic_cast<CompactHistoryScroll *>(old);
-        if (newBuffer != nullptr) {
-            newBuffer->setMaxNbLines(_maxLines);
-            return newBuffer;
-        }
-
-        newBuffer = new CompactHistoryScroll(_maxLines);
-        
-        Character line[LINE_SIZE];
-        int lines = old->getLines();
-        int i = qMax((lines - (int)_maxLines - 1), 0);
-        for (; i < lines; i++) {
-            int size = old->getLineLen(i);
-            if (size > LINE_SIZE) {
-                auto tmp_line = new Character[size];
-                old->getCells(i, 0, size, tmp_line);
-                newBuffer->addCells(tmp_line, size);
-                newBuffer->addLine(old->isWrappedLine(i));
-                delete[] tmp_line;
-            } else {
-                old->getCells(i, 0, size, line);
-                newBuffer->addCells(line, size);
-                newBuffer->addLine(old->isWrappedLine(i));
-            }
+    if (auto *newBuffer = dynamic_cast<CompactHistoryScroll *>(old.get())) {
+        newBuffer->setMaxNbLines(_maxLines);
+        return;
+    }
+    auto newScroll = std::make_unique<CompactHistoryScroll>(_maxLines);
+
+    Character line[LINE_SIZE];
+    int lines = (old != nullptr) ? old->getLines() : 0;
+    int i = qMax((lines - (int)_maxLines - 1), 0);
+    for (; i < lines; i++) {
+        int size = old->getLineLen(i);
+        if (size > LINE_SIZE) {
+            auto tmp_line = std::make_unique<Character[]>(size);
+            old->getCells(i, 0, size, tmp_line.get());
+            newScroll->addCells(tmp_line.get(), size);
+            newScroll->addLine(old->isWrappedLine(i));
+        } else {
+            old->getCells(i, 0, size, line);
+            newScroll->addCells(line, size);
+            newScroll->addLine(old->isWrappedLine(i));
         }
-
-        delete old;
-        return newBuffer;
     }
-    return new CompactHistoryScroll(_maxLines);
+    old = std::move(newScroll);
 }

src/history/compact/CompactHistoryType.h

@@ -21,7 +21,7 @@ public:
     bool isEnabled() const override;
     int maximumLineCount() const override;
 
-    HistoryScroll *scroll(HistoryScroll *) const override;
+    void scroll(std::unique_ptr<HistoryScroll> &) const override;
 
 protected:
     unsigned int _maxLines;