[utilities/krusader] app/BookMan: BookMan: Solve 'After adding the same bookmark multiple times, only one instance is displayed and later Krusader ends abruptly'

Git commit f06b8f0f969d45093129387fcb45be847ef832f9 by Toni Asensi Esteve.
Committed on 16/08/2026 at 22:14.
Pushed by asensi into branch 'master'.

BookMan: Solve 'After adding the same bookmark multiple times, only one instance is displayed and later Krusader ends abruptly'

Solve a mismatch between the `krbookmarks.xml` format (which allows
multiple bookmarks to use the same display name) and KActionCollection
(which uses string names in order to identify pointers).

Now, the `importFromFileBookmark` function checks the `seenBookmarks`
QSet. If an action name has already been claimed during the current
parsing pass, it automatically generates a unique internal suffix
(e.g. `_2`, `_3`).

This allows Krusader to safely allocate distinct KrBookmark pointers
for every entry in the XML file, preserving user data and avoiding
use-after-free and double-free problems.

FIXED: [ 524076 ] After adding the same bookmark multiple times, only one instance is displayed and later Krusader ends abruptly
BUG: 524076

Revision: https://invent.kde.org/utilities/krusader/-/merge_requests/193

M  +30   -12   app/BookMan/krbookmarkhandler.cpp

https://invent.kde.org/utilities/krusader/-/commit/f06b8f0f969d45093129387fcb45be847ef832f9

diff --git a/app/BookMan/krbookmarkhandler.cpp b/app/BookMan/krbookmarkhandler.cpp
index 6e96da792..807ab5462 100644
--- a/app/BookMan/krbookmarkhandler.cpp
+++ b/app/BookMan/krbookmarkhandler.cpp
@@ -274,20 +274,38 @@ bool KrBookmarkHandler::importFromFileBookmark(QDomElement &e, KrBookmark *paren
     if (e.hasAttribute("icon")) {
         iconName = e.attribute("icon");
     }
-    // ok: got name and url, let's add a bookmark
-    KrBookmark *bm = KrBookmark::getExistingBookmark(path + name, _collection);
-    if (!bm) {
-        bm = new KrBookmark(name, QUrl(url), _collection, iconName, path + name);
-    } else {
-        bm->setURL(QUrl(url));
-        bm->setIconName(iconName);
+    // ok: got name and url, let's add a bookmark safely allowing duplicates
+    KrBookmark *bm = nullptr;
+    QString baseActionName = path + name;
+    QString actionName = baseActionName;
+    int suffix = 1;
+
+    // Loop to find an unused internal action name. This allows having
+    // multiple bookmarks with the exact same display name
+    // to exist as distinct pointers
+    while (true) {
+        bm = KrBookmark::getExistingBookmark(actionName, _collection);
+        if (!bm) {
+            // No bookmark exists with this internal actionName.
+            // Create a new bookmark safely
+            bm = new KrBookmark(name, QUrl(url), _collection, iconName, actionName);
+            break;
+        } else if (!seenBookmarks.contains(bm)) {
+            // We have found an existing one that hasn't been added to
+            // the tree yet in this pass. Reuse it
+            bm->setURL(QUrl(url));
+            bm->setIconName(iconName);
+            break;
+        }
+        // This pointer was already used (it's a duplicate name in the XML).
+        // Increment the suffix and check for (or create) a unique internal action name
+        suffix++;
+        actionName = baseActionName + "_" + QString::number(suffix);
     }
 
-    // Prevent duplicated pointers in the tree (in order to stop double-frees)
-    if (!seenBookmarks.contains(bm)) {
-        parent->children().append(bm);
-        seenBookmarks.insert(bm);
-    }
+    // Append it to the tree and record it in the guard set
+    parent->children().append(bm);
+    seenBookmarks.insert(bm);
 
     return true;
 }