[sdk/kommit] src/libkommit: libkommit: let Qt own the memory of a string array

Git commit ea92ea1508fd5ad2169c0dc6dc552f659ae70104 by Hamed Masafi, on behalf of Méven Car.
Committed on 17/08/2026 at 14:59.
Pushed by hamedmasafi into branch 'master'.

libkommit: let Qt own the memory of a string array

StrArray hands libgit2 a git_strarray, an array of UTF-8 pointers, and did its own
allocation to build one. Several things were wrong with it: add() asked for more
room only when there was room already, so the first string added to a full array
was written one past the last slot; it then raised the capacity rather than the
count, leaving the count at the number of slots asked for rather than the number of
strings in them; one constructor allocated with new[] and freed with free();
another read an uninitialised pointer before setting it. Nothing had called add()
yet, so none of it had been noticed.

The class now keeps the UTF-8 bytes of each string in a QList<QByteArray> and the
pointers into them in a QList<char *>, both of which Qt looks after, and hands the
second one to libgit2. It cannot be copied, since what libgit2 is given points into
it. A QStringList cannot serve directly: its strings are UTF-16 and it keeps no
array of pointers.

M  +26   -65   src/libkommit/entities/strarray.cpp
M  +20   -16   src/libkommit/entities/strarray.h
M  +1    -2    src/libkommit/fetch.cpp

https://invent.kde.org/sdk/kommit/-/commit/ea92ea1508fd5ad2169c0dc6dc552f659ae70104

diff --git a/src/libkommit/entities/strarray.cpp b/src/libkommit/entities/strarray.cpp
index b5a61eb8..970876bf 100644
--- a/src/libkommit/entities/strarray.cpp
+++ b/src/libkommit/entities/strarray.cpp
@@ -3,93 +3,54 @@
 namespace Git
 {
 
-StrArray::StrArray(size_t initialSize)
-    : capacity{}
+StrArray::StrArray(const QStringList &strings)
 {
-    char **newStrings = static_cast<char **>(malloc(initialSize * sizeof(char *)));
-    if (newStrings) {
-        strarray.count = initialSize;
-        strarray.strings = newStrings;
-    }
+    setStrings(strings);
 }
 
-StrArray::StrArray(const QString &item)
+StrArray::StrArray(const QString &string)
+    : StrArray{QStringList{string}}
 {
-    strarray.strings = new char *[1];
-    strarray.strings[0] = strdup(item.toUtf8().constData());
-    strarray.count = 1;
 }
 
-StrArray::StrArray(const QStringList &list)
+void StrArray::setStrings(const QStringList &strings)
 {
-    fromQStringList(list);
-}
+    mUtf8.clear();
+    mPointers.clear();
+    mUtf8.reserve(strings.size());
+    mPointers.reserve(strings.size());
 
-StrArray::~StrArray()
-{
-    clear();
-}
+    for (const auto &string : strings)
+        mUtf8 << string.toUtf8();
 
-void StrArray::reserve(size_t size)
-{
-    if (size > capacity) {
-        char **newStrings = static_cast<char **>(realloc(strarray.strings, size * sizeof(char *)));
-        if (newStrings) {
-            strarray.count = size;
-            strarray.strings = newStrings;
-            capacity = size;
-        }
-    }
-}
+    // Filled after every byte array is in place: appending to mUtf8 can move what is in it,
+    // and a pointer taken before that would be left behind.
+    for (auto &bytes : mUtf8)
+        mPointers << bytes.data();
 
-void StrArray::add(const QString &str)
-{
-    if (strarray.count < capacity) {
-        reserve(strarray.count + 1); // Reserve exactly the needed size
-    }
-    strarray.strings[strarray.count] = strdup(str.toUtf8().constData());
-    capacity++;
+    mArray.strings = mPointers.data();
+    mArray.count = static_cast<size_t>(mPointers.size());
 }
 
-void StrArray::fromQStringList(const QStringList &list)
+QStringList StrArray::strings() const
 {
-    clear();
-    reserve(list.size());
-    for (int i = 0; i < list.size(); ++i) {
-        add(list[i]);
-    }
-}
+    QStringList strings;
+    strings.reserve(mUtf8.size());
 
-QStringList StrArray::toQStringList() const
-{
-    QStringList list;
-    for (size_t i = 0; i < strarray.count; ++i) {
-        list.append(QString::fromUtf8(strarray.strings[i]));
-    }
-    return list;
+    for (const auto &bytes : mUtf8)
+        strings << QString::fromUtf8(bytes);
+
+    return strings;
 }
 
 StrArray::operator git_strarray *()
 {
-    return &strarray;
+    return &mArray;
 }
 
 const git_strarray *StrArray::operator*() const
 {
-    return &strarray;
-}
-
-void StrArray::clear()
-{
-    if (strarray.strings) {
-        for (size_t i = 0; i < strarray.count; ++i) {
-            free(strarray.strings[i]);
-        }
-        free(strarray.strings);
-        strarray.strings = nullptr;
-        strarray.count = 0;
-        capacity = 0;
-    }
+    return &mArray;
 }
 
 }
diff --git a/src/libkommit/entities/strarray.h b/src/libkommit/entities/strarray.h
index da1de05a..f58ff3f4 100644
--- a/src/libkommit/entities/strarray.h
+++ b/src/libkommit/entities/strarray.h
@@ -1,38 +1,42 @@
 #pragma once
 
+#include <QByteArray>
+#include <QList>
 #include <QString>
 #include <QStringList>
-#include <cstdlib> // for malloc, realloc, free
-#include <cstring> // for strdup
+
 #include <git2.h>
 
 namespace Git
 {
 
+/**
+ * A list of strings in the form libgit2 reads them, a git_strarray of UTF-8 pointers.
+ *
+ * It holds the UTF-8 bytes of each string, and the pointers into those bytes, for as long as
+ * the call that reads them lasts. Qt owns the memory, so there is nothing here to allocate or
+ * free by hand.
+ */
 class StrArray
 {
 public:
-    explicit StrArray(size_t initialSize = 0);
-    explicit StrArray(const QString &item);
-    explicit StrArray(const QStringList &list);
-
-    ~StrArray();
+    explicit StrArray(const QStringList &strings = {});
+    explicit StrArray(const QString &string);
 
-    void reserve(size_t size);
+    // What libgit2 is given points into this object, so it stays where it was made.
+    StrArray(const StrArray &) = delete;
+    StrArray &operator=(const StrArray &) = delete;
 
-    void add(const QString &str);
-
-    void fromQStringList(const QStringList &list);
-    [[nodiscard]] QStringList toQStringList() const;
+    void setStrings(const QStringList &strings);
+    [[nodiscard]] QStringList strings() const;
 
     explicit operator git_strarray *();
     const git_strarray *operator*() const;
 
-    void clear();
-
 private:
-    git_strarray strarray;
-    size_t capacity;
+    QList<QByteArray> mUtf8;
+    QList<char *> mPointers;
+    git_strarray mArray{nullptr, 0};
 };
 
 }
diff --git a/src/libkommit/fetch.cpp b/src/libkommit/fetch.cpp
index f8b41169..32575777 100644
--- a/src/libkommit/fetch.cpp
+++ b/src/libkommit/fetch.cpp
@@ -79,8 +79,7 @@ int FetchPrivate::run()
 
     int ret;
     if (!branch.isNull()) {
-        StrArray refSpecs{1};
-        refSpecs.add(branch.refName());
+        StrArray refSpecs{branch.refName()};
         ret = SequenceRunner::runSingle(git_remote_fetch, remote.remotePtr(), *refSpecs, &opts, "fetch");
     } else {
         ret = SequenceRunner::runSingle(git_remote_fetch, remote.remotePtr(), (const git_strarray *)NULL, &opts, "fetch");