<div dir="ltr">Inside qofinstance.cpp, the guid functionality is only used inside the following four functions.  Those only seem to be used by the Split.cpp (in particular Peer splits.) The add_guid only handle the kvp frame not the list.  The others work with either.<br><br>1. qof_instance_kvp_add_guid --- only used in split: xaccSplitAddPeerSplit<br>   Only does kvpframe version.<br>2. qof_instance_kvp_has_guid  --- Only used in split: xaccSplitIsPeerSplit<br>3. qof_instance_kvp_remove_guid --- Only used in split: xaccSplitRemovePeerSplit<br>4. qof_instance_kvp_merge_guids --- Only used in xaccSplitMergePeerSplits<br><br><br>I have removed the glist parts of the other three methods, and run the full test suite. There were no failed tests.<br><br>I'm glad you suggested removing the glist code.  I was kinda stuck on setting up an object manually just to test the other functions.  It is not worth it, unless there is some concern that some real code is doing that. (I'm not sure I know how to look for that, so have not.)<br><br>I will add the glist removal to the pending pull request (after I get the rest of qofinstance.cpp tested).<br><br><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Tue, Jan 27, 2026 at 1:18 PM John Ralls <<a href="mailto:jralls@ceridwen.us">jralls@ceridwen.us</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
> On Jan 27, 2026, at 04:55, Stefan Koch <<a href="mailto:stefan.koch.micro@gmail.com" target="_blank">stefan.koch.micro@gmail.com</a>> wrote:<br>
> <br>
> In the KvpValue::Type::FRAME case if the guid matches, it does both:<br>
>             delete inst->kvp_data->set_path({path}, nullptr);<br>
>             delete v;<br>
> Where earlier the v was set to be:<br>
>     auto v = inst->kvp_data->get_slot({path});<br>
> I think the second delete is a duplicate of the first and should be removed.<br>
> <br>
> I found it by getting a strange warning and then a crash on that second delete. I verified that it calls the frame destructor on the same object twice. It is not surprising that double delete causes strange behavior, but I did not verify the exact problem.<br>
> <br>
> The test case that crashed was:<br>
>     auto gncGuid1 = guid_new();<br>
>     qof_instance_kvp_add_guid(m_inst, "guid", 123u, "mytime", gncGuid1);<br>
>     qof_instance_kvp_remove_guid(m_inst, "guid", "mytime", gncGuid1);<br>
> <br>
> <br>
> This functionality is used in Split.cpp in support of xaccScrubMergeLotSubSplits().  Where and why that is used, I am not sure, but it is likely to be a problem under circumstances where the code is actually called. I'm not sure if this is common, or even possible use case.<br>
> <br>
> I think the following will fix this issue:<br>
> modified   libgnucash/engine/qofinstance.cpp<br>
> @@ -1216,7 +1216,6 @@ qof_instance_kvp_remove_guid (const QofInstance *inst, const char *path,<br>
>          if (kvp_match_guid (v, {key}, guid))<br>
>          {<br>
>              delete inst->kvp_data->set_path({path}, nullptr);<br>
> -            delete v;<br>
>          }<br>
>          break;<br>
>      case KvpValue::Type::GLIST:<br>
> <br>
> <br>
> Since I am new, I wanted to get confirmation, that I am not missing something.<br>
> <br>
> If the change is good, I will add it (as a separate commit) to my next testing merge request.  That is what we agreed to on the previous, less serious, issue I found.<br>
<br>
Stefan,<br>
<br>
Yes, it’s a double free and removing the second delete is the correct fix.<br>
<br>
The GLIST branch of those functions was for backwards compatibility but I don’t think that I did it right. I don't think it’s reachable with current code unless you craft a test-only function to create a GLIST KVP that has a GUID element in it.. If that proves out then remove the switch and just test that the KVP value returned from the path is a KvpFrame*.<br>
<br>
Regards,<br>
John Ralls<br>
<br>
<br>
<br>
</blockquote></div>