> On Jan 28, 2026, at 05:35, Stefan Koch <[email protected]> wrote: > > There is one more issue related to this in qof_instance_kvp_merge_guids here: > if (target_val) > target_val->add(v); > > This does not work if the target_val is not already a glist. In that case, > the KvpValueImpl::add creates a new GList which it returns, that the above > code ignores. (If it was already a list it should work OK.). > > Since the qof_instance_kvp_merge_guids is only used in the peer splits we > looked at yesterday, I think it is safe (readonable?) to assume that it will > not use the glist functionality here. > > I will add the following change to the removal of glist cases in qofinstance > guid functionality. > > modified libgnucash/engine/qofinstance.cpp > @@ -1220,7 +1220,7 @@ qof_instance_kvp_merge_guids (const QofInstance *target, > if(v->get_type() == KvpValue::Type::FRAME) > { > if (target_val) > - target_val->add(v); > + PWARN ("Instance KVP on path %s already exists.", path); > else > target->kvp_data->set_path({path}, v); > donor->kvp_data->set({path}, nullptr); //Contents moved, Don't > delete! > > This still leaves the tricky to use KvpValueImpl::add. But it turns out that > this is only used here, and so when I remove it here, it has no other uses > (besides the unit test). I will delete that functionality in the same > commit. > > I have created the pull request for this change (with the trivial idata > property fix as a previous commit).
Stefan, No, KvpVallue::add does work if it doesn’t contain a GLIst: It creates a new KvpValue of type GLIST and populates its Glist with the original value and the new one. It doesn’t matter that gnc_instance_kvp_merge_guid discards the returned KvpValue* because it doesn’t have any use for it: What’s important is that the Split’s peer slot has the new GLIST KvpValue and subsequent calls to retrieve that slot or KVP path will get it. This also changes the equation for testing because it means that there is a way to get a GUID-GLIST Kvp that you can use to exercise the GLIST branches of gnc_instance_kvp_has_guid and gnc_instance_kvp_remove_guid. Looks like we need better capgains testing before we muck with this code. Regards, John Ralls _______________________________________________ gnucash-devel mailing list [email protected] https://lists.gnucash.org/mailman/listinfo/gnucash-devel
