diff --git a/src/engine/Account.c b/src/engine/Account.c index 0032520d5d..1bb65b3c0c 100644 --- a/src/engine/Account.c +++ b/src/engine/Account.c @@ -222,6 +222,7 @@ xaccAccountInsertSplit ( Account *acc, Split *split ) { int i,j; Split **oldsplits; + Account *oldacc; if (!acc) return; if (!split) return; @@ -254,39 +255,73 @@ disable for now till we figure out what the right thing is. * there first. We don't want to ever leave the system * in an inconsistent state. */ + oldacc = split->acc; if (split->acc) xaccAccountRemoveSplit (split->acc, split); split->acc = acc; - oldsplits = acc->splits; - acc->numSplits ++; - acc->splits = (Split **)_malloc(((acc->numSplits) + 1) * sizeof(Split *)); - - /* Find the insertion point */ - /* to get realy fancy, could use binary search. */ - for(i = 0; i < (acc->numSplits - 1);) { - if(xaccSplitOrder(&(oldsplits[i]), &split) > 0) { - break; - } else { - acc->splits[i] = oldsplits[i]; - } - i++; /* Don't put this in the loop guard! It'll go too far. */ + /* enlarge the size of the split array to accomadate the new split, + * and copy all the splits over to the new array. + * If the old and new accounts are the same account, then we + * are just shuffling around the split, resumably due to a + * date reordering. In this case, most of the malloc/copy/free bit + * can be avoided. + */ + if (oldacc != acc) { + oldsplits = acc->splits; + acc->numSplits ++; + + acc->splits = (Split **)_malloc(((acc->numSplits) + 1) * sizeof(Split *)); + + /* Find the insertion point */ + /* to get realy fancy, could use binary search. */ + for(i = 0; i < (acc->numSplits - 1);) { + if(xaccSplitOrder(&(oldsplits[i]), &split) > 0) { + break; + } else { + acc->splits[i] = oldsplits[i]; + } + i++; /* Don't put this in the loop guard! It'll go too far. */ + } + /* Insertion point is now i */ + + //fprintf(stderr, "Insertion position is: %d\n", i); + + /* Move all the other splits down (this could be done faster with memmove)*/ + for( j = acc->numSplits; j > i; j--) { + acc->splits[j] = oldsplits[j - 1]; + } + + /* Now insert the new split */ + acc->splits[i] = split; + + /* make sure the array is NULL terminated */ + acc->splits[acc->numSplits] = NULL; + + _free(oldsplits); + } else { + acc->numSplits ++; + + /* Find the insertion point */ + /* to get realy fancy, could use binary search. */ + for(i = 0; i < (acc->numSplits - 1);) { + if(xaccSplitOrder(&(acc->splits[i]), &split) > 0) { + break; + } + i++; /* Don't put this in the loop guard! It'll go too far. */ + } + /* Insertion point is now i */ + + /* Move all the other splits down (this could be done faster with memmove)*/ + for( j = acc->numSplits; j > i; j--) { + acc->splits[j] = acc->splits[j - 1]; + } + + /* Now insert the new split */ + acc->splits[i] = split; + + /* make sure the array is NULL terminated */ + acc->splits[acc->numSplits] = NULL; } - /* Insertion point is now i */ - - //fprintf(stderr, "Insertion position is: %d\n", i); - - /* Move all the other splits down (this could be done faster with memmove)*/ - for( j = acc->numSplits; j > i; j--) { - acc->splits[j] = oldsplits[j - 1]; - } - - /* Now insert the new split */ - acc->splits[i] = split; - - /* make sure the array is NULL terminated */ - acc->splits[acc->numSplits] = NULL; - - _free(oldsplits); xaccAccountRecomputeBalance (acc); } @@ -485,7 +520,6 @@ xaccCheckDateOrder (Account * acc, Split *split ) /* take care of re-ordering, if necessary */ if( outOfOrder ) { - xaccAccountRemoveSplit( acc, split ); xaccAccountInsertSplit( acc, split ); return 1; } diff --git a/src/engine/Transaction.c b/src/engine/Transaction.c index e6694fa93e..0740bde544 100644 --- a/src/engine/Transaction.c +++ b/src/engine/Transaction.c @@ -414,6 +414,11 @@ xaccFreeTransaction( Transaction *trans ) trans->open = 0; + if (trans->orig) { + xaccFreeTransaction (trans->orig); + trans->orig = NULL; + } + _free(trans); } @@ -767,12 +772,6 @@ xaccTransCommitEdit (Transaction *trans) if (!trans) return; CHECK_OPEN (trans); - /* get rid of the copy we made. We won't be rolling back, - * so we don't need i any more. - */ - xaccFreeTransaction (trans->orig); - trans->orig = NULL; - /* At this point, we check to see if we have a valid transaction. * As a result of editing, we could end up with a transaction that * has no splits in it, in which case we delete the transaction and @@ -807,17 +806,26 @@ xaccTransCommitEdit (Transaction *trans) trans->open &= ~DEFER_REBALANCE; xaccTransRebalance (trans); - /* um, theoritically, it is impossible for splits - * to get inserted out of order. But we'll get paranoid, - * and check anyway, at the loss of some performance. - */ - i=0; - split = trans->splits[i]; - while (split) { - acc = split ->acc; - xaccCheckDateOrder(acc, trans->splits[i]); - i++; + /* check to see if the date has changed. We use the date as the sort key. */ + if ((trans->orig->date_entered.tv_sec != trans->date_entered.tv_sec) || + (trans->orig->date_posted.tv_sec != trans->date_posted.tv_sec)) + { + + /* since the date has changed, we need to be careful to + * make sure all associated splits are in proper order + * in thier accounts. The easiest way of ensuring this + * is to remove and reinsert every split. The reinsertion + * process will place the split in the correct date-sorted + * order. + */ + i=0; split = trans->splits[i]; + while (split) { + acc = split ->acc; + xaccCheckDateOrder(acc, trans->splits[i]); + i++; + split = trans->splits[i]; + } } i=0; @@ -831,6 +839,12 @@ xaccTransCommitEdit (Transaction *trans) trans->open = 0; xaccTransWriteLog (trans, 'C'); + + /* get rid of the copy we made. We won't be rolling back, + * so we don't need it any more. */ + xaccFreeTransaction (trans->orig); + trans->orig = NULL; + } void @@ -1203,40 +1217,22 @@ xaccCountTransactions (Transaction **tarray) void xaccTransSetDateSecs (Transaction *trans, time_t secs) { - Split *split; - Account *acc; - int i=0; - if (!trans) return; CHECK_OPEN (trans); /* hack alert -- for right now, keep the posted and the entered * dates in sync. Later, we'll have to split these up. */ - trans->date_entered.tv_sec = secs; trans->date_posted.tv_sec = secs; - /* since the date has changed, we need to be careful to - * make sure all associated splits are in proper order - * in thier accounts. The easiest way of ensuring this - * is to remove and reinsert every split. The reinsertion - * process will place the split in the correct date-sorted - * order. + /* Because the date has changed, we need to make sure that each of the + * splits is properly ordered in each of thier accounts. We could do that + * here, simply by reinserting each split into its account. However, in + * some ways this is bad behaviour, and it seems much better/nicer to defer + * that until the commit phase, i.e. until the user has called the + * xaccTransCommitEdit() routine. So, for now, we are done. */ - - assert (trans->splits); - - i=0; - split = trans->splits[i]; - while (split) { - acc = split->acc; - xaccAccountRemoveSplit (acc, split); - xaccAccountInsertSplit (acc, split); - - i++; - split = trans->splits[i]; - } } void