bad API design (was: Budgets ... again)

[Christian Stimming]

Am Montag, 17. Oktober 2011 schrieb John Ralls:
> On Sep 26, 2011, at 11:35 AM, Christian Stimming wrote:
> > Am Montag, 26. September 2011 schrieb John Ralls:
> >> I have a plan to rework the engine (the module which does all of
> >> the accounting and business calculations) in stages. Stage one is to
> >> write comprehensive tests for each class. Stage two (which will
> >> interleave with stage one) is to overhaul each class to be a
> >> well-behaved GObject class with all access to data (including KVP) via
> >> function calls instead of passing out pointers for other objects to
> >> modify at will.
> > 
> > Out of curiosity: Which examples for "passing out pointers" do you have
> > in mind here? Surely there is plenty of access to KVP data by simply
> > accessing the KVP directly, which is bad, but is there also direct
> > pointer manipulation in that many places?
> 
> Here's an example I found today, while investigating bug 661903:
> xaccTransGetSplitList() returns a GList* of the splits. The requestor can
> manipulate or delete the splits, unbalancing or even eviscerating the
> transaction without the transaction knowing. It's called in 23 places in C
> and 19 in Scheme.
> 
> It doesn't help that Accounts also keep a list of splits and since we're
> not using GObject reference counting, there's no way to protect against
> one or both from having invalid pointers.

Absolutely. All of the API functions passing out a GList* are prone to this 
error. However, that's an inherent issue of any C API, because the only 
alternative is to pass out a newly allocated GList* each time. You can easily 
guess this will lead to plenty of memory leaks... well... thinking about it 
again, this might not be as bad as it sounds. Maybe those functions should 
indeed pass out a newly allocated GList instead of the internal one, i.e. just 
using g_list_copy. Then again, the data pointers will still be plain non-const 
pointers as there isn't a const variant of GList*...

Regards,

Christian