Possible bug in qofinstance.cpp: qof_instance_kvp_remove_guid

[John Ralls]

> On Jan 27, 2026, at 12:19, Stefan Koch <stefan.koch.micro at gmail.com> wrote:
> 
> 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.
> 
> 1. qof_instance_kvp_add_guid --- only used in split: xaccSplitAddPeerSplit
>    Only does kvpframe version.
> 2. qof_instance_kvp_has_guid  --- Only used in split: xaccSplitIsPeerSplit
> 3. qof_instance_kvp_remove_guid --- Only used in split: xaccSplitRemovePeerSplit
> 4. qof_instance_kvp_merge_guids --- Only used in xaccSplitMergePeerSplits
> 
> 
> I have removed the glist parts of the other three methods, and run the full test suite. There were no failed tests.
> 
> 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.)
> 
> I will add the glist removal to the pending pull request (after I get the rest of qofinstance.cpp tested).
> 

Stefan,

No test touches qof_instance_kvp_remove_guid or qof_instance_kvp_merge_guids, see https://gnucash.github.io/gnucash/Coverage-HTML/libgnucash/engine/qofinstance.cpp.gcov.html, so it doesn’t provide any assurance that my analysis is correct. I think it’s a pretty safe bet anyway given that the functions are all called from only one place.

I realized in reviewing that code that it assumes that a split should have only one peer relationship at a time, and that’s not necessarily true. 
So I went to search in BZ to see if anyone had reported a problem with that (didn’t find any, but...) I found bugs.gnucash.org/show_bug.cgi?id=798873 <http://bugs.gnucash.org/show_bug.cgi?id=798873>, and there’s a stack trace showing the crash originates at qofinstance.cpp line 1188. The version of qofinstance.cpp in 5.0, the GnuCash version of the bug, is https://github.com/Gnucash/gnucash/blob/af02dae28684f1e31c6937dc5a30df4d0e7adb01/libgnucash/engine/qofinstance.cpp and line 1188 is the second deletion of v in qof_instance_kvp_remove_guid, so you get to claim your first bug fix! The protocol for bug fix commits is that the summary line should be a copy of the bug title, in this case "Bug 798873 - Crash when scrubbing after "undoing” changes”.  Please use that for your commit.

Regards,
John Ralls


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gnucash.org/pipermail/gnucash-devel/attachments/20260127/526c7635/attachment.htm>