diff --git a/src/engine/FileIO.c b/src/engine/FileIO.c index da9115b639..a757f18d2a 100644 --- a/src/engine/FileIO.c +++ b/src/engine/FileIO.c @@ -94,6 +94,7 @@ #include "messages.h" #include "Transaction.h" #include "TransactionP.h" +#include "TransLog.h" #include "util.h" #define PERMS 0666 @@ -242,6 +243,8 @@ xaccReadAccountGroup( char *datafile ) return NULL; } + /* disable logging during load; otherwise its just a mess */ + xaccLogDisable(); holder = xaccMallocAccountGroup(); grp = readGroup (fd, NULL, token); @@ -267,6 +270,7 @@ xaccReadAccountGroup( char *datafile ) maingrp = NULL; + xaccLogEnable(); close(fd); return grp; } @@ -396,6 +400,20 @@ readAccount( int fd, AccountGroup *grp, int token ) printf (" expected %d got %d transactions \n",numTrans,i); break; } +#ifdef DELINT_BLANK_SPLITS_HACK + /* This is a dangerous hack, as it can destroy real data. */ + { + int j=0; + Split *s = trans->splits[0]; + while (s) { + if (DEQ(0.0,s->damount) && DEQ (1.0,s->share_price)) { + xaccTransRemoveSplit (trans, s); + break; + } + j++; s=trans->splits[j]; + } + } +#endif /* DELINT_BLANK_SPLITS_HACK */ } springAccount (acc->id); @@ -558,6 +576,13 @@ readTransaction( int fd, Account *acc, int token ) if (4 >= token) { Split * s; + + /* The code below really wants to assume that there are a pair + * of splits in every transaction, so make it so. + */ + s = xaccMallocSplit (); + xaccTransAppendSplit (trans, s); + tmp = readString( fd, token ); if( NULL == tmp ) { @@ -569,7 +594,8 @@ readTransaction( int fd, Account *acc, int token ) free (tmp); /* action first introduced in version 3 of the file format */ - if (3 <= token) { + if (3 <= token) + { tmp = readString( fd, token ); if( NULL == tmp ) { @@ -741,8 +767,8 @@ readTransaction( int fd, Account *acc, int token ) XACC_FLIP_INT (numSplits); for (i=0; i i+offset) { - /* the first two splits have been malloced. just replace them */ + if (0 == i+offset) { + /* the first split has been malloced. just replace it */ xaccFreeSplit (trans->splits[i+offset]); trans->splits[i+offset] = split; split->parent = trans; diff --git a/src/engine/TransLog.c b/src/engine/TransLog.c index 9fbb3f4f38..c1798173ef 100644 --- a/src/engine/TransLog.c +++ b/src/engine/TransLog.c @@ -35,14 +35,60 @@ #include "util.h" /* - * The logfiles are useful for tracing, journalling. + * The logfiles are useful for tracing, journalling, error recovery. * Note that the current support for journalling is at best - * embryonic, at worst, sets the wrong expectations. + * embryonic, at worst, is dangerous by setting the wrong expectations. */ -int gen_logs = 1; -FILE * trans_log = 0x0; -FILE * split_log = 0x0; +/* + * Some design philosphy that I think would be good to keep in mind: + * (0) Simplicity and foolproofness are the over-riding design points. + * This is supposed to be a fail-safe safety net. We don't want + * our safety net to fail because of some whiz-bang shenanigans. + * + * (1) Try to keep the code simple. Want to make it simple and obvious + * that we are recording everything that we need to record. + * + * (2) Keep the printed format human readable, for the same reasons. + * (2.a) Keep the format, simple, flat, more or less unstructured, + * record oriented. This will help parsing by perl scripts. + * No, using a perl script to analyze a file that's supposed to be human + * readable is not a contradication in terms -- that's exactly the point. + * (2.b) Use tabs as a human freindly field separator; its also a character + * that does not (should not) appear naturally anywhere in the data, + * as it serves no formatting purpose in the current GUI design. + * (hack alert -- this is not currently tested for or enforced, + * so this is a very unsafe assumption. Maybe urlencoding should + * be used.) + * (2.c) Don't print redundant information in a single record. This would + * just confuse any potential user of this file. + * (2.d) Saving space, being compact is not a priority, I don't think. + * + * (3) There are no compatibility requirements from release to release. + * Sounds OK to me to change the format of the output when needed. + * + * (-) print transaction start and end delimiters + * (-) print a unique transaction id as a handy label for anyone + * who actually examines these logs. + * The C address pointer to the transaction struct should be fine, + * as it is simple and unique until the transaction is deleted ... + * and we log deletions, so that's OK. Just note that the id + * for a deleted transaction might be recycled. + * (-) print the current timestamp, so that if it is known that a bug + * occurred at a certain time, it can be located. + * (-) hack alert -- something better than just the account name + * is needed for identifying the account. + */ + + +static int gen_logs = 1; +static FILE * trans_log = 0x0; + +/********************************************************************\ +\********************************************************************/ + +void xaccLogDisable (void) { gen_logs = 0; } +void xaccLogEnable (void) { gen_logs = 1; } /********************************************************************\ \********************************************************************/ @@ -50,41 +96,29 @@ FILE * split_log = 0x0; void xaccOpenLog (void) { + char filename[1000]; char * timestamp; if (!gen_logs) return; - if (trans_log && split_log) return; + if (trans_log) return; /* tag each filename with a timestamp */ timestamp = xaccDateUtilGetStampNow (); - if (!trans_log) { - char filename[1000]; + strcpy (filename, "translog."); + strcat (filename, timestamp); + strcat (filename, ".log"); - strcpy (filename, "translog."); - strcat (filename, timestamp); - strcat (filename, ".log"); + trans_log = fopen (filename, "a"); - trans_log = fopen (filename, "a"); + /* use tab-separated fields */ + fprintf (trans_log, "mod id time_now " \ + "date_entered date_posted " \ + "num description " \ + "account memo action reconciled " \ + "amount price date_reconciled\n"); + fprintf (trans_log, "-----------------\n"); - /* use tab-separated fields, to be /rdb compatible */ - fprintf (trans_log, "num description\n"); - fprintf (trans_log, "-----------------\n"); - } - - if (!split_log) { - char filename[1000]; - - strcpy (filename, "splitlog."); - strcat (filename, timestamp); - strcat (filename, ".log"); - - split_log = fopen (filename, "a"); - - /* use tab-separated fields, to be /rdb compatible */ - fprintf (split_log, "num memo action reconciled amount price\n"); - fprintf (split_log, "-----------------\n"); - } free (timestamp); } @@ -92,34 +126,56 @@ xaccOpenLog (void) \********************************************************************/ void -xaccTransWriteLog (Transaction *trans) +xaccTransWriteLog (Transaction *trans, char flag) { Split *split; int i = 0; + char *dnow, *dent, *dpost, *drecn; if (!gen_logs) return; - if (!trans_log || !split_log) return; + if (!trans_log) return; - /* use tab-separated fields, to be /rdb compatible */ - fprintf (trans_log, "%s %s\n", trans->num, trans->description); + dnow = xaccDateUtilGetStampNow (); + dent = xaccDateUtilGetStamp (trans->date_entered.tv_sec); + dpost = xaccDateUtilGetStamp (trans->date_posted.tv_sec); + + fprintf (trans_log, "===== START\n"); split = trans->splits[0]; while (split) { - fprintf (split_log, "%s %s %s %c %10.6f %10.6f\n", - trans->num, + char * accname = ""; + if (split->acc) accname = split->acc->description; + drecn = xaccDateUtilGetStamp (split->date_reconciled.tv_sec); + + /* use tab-separated fields */ + fprintf (trans_log, "%c %p %s %s %s %s %s " \ + "%s %s %s %c %10.6f %10.6f %s\n", + flag, + trans, + dnow, + dent, + dpost, + trans->num, + trans->description, + accname, split->memo, split->action, split->reconciled, split->damount, - split->share_price + split->share_price, + drecn ); + free (drecn); i++; split = trans->splits[i]; } + fprintf (trans_log, "===== END\n"); + free (dnow); + free (dent); + free (dpost); /* get data out to the disk */ fflush (trans_log); - fflush (split_log); } /********************************************************************\ @@ -127,6 +183,8 @@ xaccTransWriteLog (Transaction *trans) #if 0 /* open_memstream seems to give various distros fits + * this has resulted in warfare on the mailing list. + * I think the truce called required changing this to asprintf * this code is not currently used ... so its ifdef out */ diff --git a/src/engine/TransLog.h b/src/engine/TransLog.h index 5e18f452a4..b1b658483d 100644 --- a/src/engine/TransLog.h +++ b/src/engine/TransLog.h @@ -27,7 +27,9 @@ #include "Transaction.h" void xaccOpenLog (void); -void xaccTransWriteLog (Transaction *); +void xaccTransWriteLog (Transaction *, char); +void xaccLogEnable (void); +void xaccLogDisable (void); /* returned strings will have been allocated with malloc, free with free() */ char *xaccSplitAsString(Split *s, const char prefix[]); diff --git a/src/engine/Transaction.c b/src/engine/Transaction.c index 23ebec06df..f26ed5d804 100644 --- a/src/engine/Transaction.c +++ b/src/engine/Transaction.c @@ -39,12 +39,21 @@ /* - * If the "force_double_entry" flag has a non-zero value, - * then all transactions will be *forced* to balance. - * This will be forced even if it requires a new split - * to be created. + * If the "force_double_entry" flag determines how + * the splits in a transaction will be balanced. + * + * The following values have significance: + * 0 -- anything goes + * 1 -- The sum of all splits in a transaction will be' + * forced to be zero, even if this requires the + * creation of additonal splits. Note that a split + * whose value is zero (e.g. a stock price) can exist + * by itself. Otherwise, all splits must come in at + * least pairs. + * 2 -- splits without oparents will be forced into a + * lost & found account. (Not implemented) */ -int force_double_entry = 0; +int force_double_entry = 1; /********************************************************************\ * Because I can't use C++ for this project, doesn't mean that I * @@ -190,11 +199,13 @@ printf ("SplitDestroy(): trans=%p, %d'th split=%p\n", trans, numsplits, s); xaccAccountRecomputeBalance (acc); s = trans->splits[1]; - acc = s->acc; - MARK_SPLIT (s); - xaccAccountRemoveSplit (acc, s); - xaccAccountRecomputeBalance (acc); - xaccFreeTransaction (trans); + if (s) { + acc = s->acc; + MARK_SPLIT (s); + xaccAccountRemoveSplit (acc, s); + xaccAccountRecomputeBalance (acc); + xaccFreeTransaction (trans); + } } } @@ -301,16 +312,13 @@ xaccInitTransaction( Transaction * trans ) trans->splits = (Split **) _malloc (3* sizeof (Split *)); - /* create a pair of splits */ + /* create a single split only. As soon as the balance becomes + * non-zero, additional splits will get created. + */ split = xaccMallocSplit (); split->parent = trans; trans->splits[0] = split; - - split = xaccMallocSplit (); - split->parent = trans; - trans->splits[1] = split; - - trans->splits[2] = NULL; + trans->splits[1] = NULL; trans->date_entered.tv_sec = 0; trans->date_entered.tv_nsec = 0; @@ -376,31 +384,11 @@ xaccFreeTransaction( Transaction *trans ) /********************************************************************\ \********************************************************************/ -void -xaccTransDestroy (Transaction *trans) -{ - int i; - Split *split; - Account *acc; - - if (!trans) return; - - i=0; - split = trans->splits[i]; - while (split) { - MARK_SPLIT (split); - acc = split ->acc; - xaccAccountRemoveSplit (acc, split); - xaccAccountRecomputeBalance (acc); - i++; - split = trans->splits[i]; - } - - xaccFreeTransaction (trans); -} - -/********************************************************************\ -\********************************************************************/ +/* hack alert -- the algorithm used in this rebalance routine + * is less than intuitive, and could use some write-up. + * Maybe it does indeed do the right thing, but that is + * not at all obvious. + */ void xaccTransRebalance (Transaction * trans) @@ -459,7 +447,7 @@ xaccSplitRebalance (Split *split) MARK_SPLIT (s); xaccAccountRecomputeBalance (s->acc); - } else{ + } else { /* There are no destination splits !! * Either this is allowed, in which case * we just blow it off, or its forbidden, @@ -467,27 +455,25 @@ xaccSplitRebalance (Split *split) * to be created. */ -/* hack alert -- I think this is broken */ -#ifdef HACK_ALERT if (force_double_entry) { - value = split->share_price * split->damount; - - /* malloc a new split, mirror it to the source split */ - s = xaccMallocSplit (); - s->damount = -value; - free (s->memo); - s->memo = strdup (split->memo); - free (s->action); - s->action = strdup (split->action); - - /* insert the new split into the transaction and - * the same account as the source split */ - MARK_SPLIT (s); - xaccTransAppendSplit (trans, s); - xaccAccountInsertSplit (split->acc, s); + if (! (DEQ (0.0, split->damount))) { + value = split->share_price * split->damount; + + /* malloc a new split, mirror it to the source split */ + s = xaccMallocSplit (); + s->damount = -value; + free (s->memo); + s->memo = strdup (split->memo); + free (s->action); + s->action = strdup (split->action); + + /* insert the new split into the transaction and + * the same account as the source split */ + MARK_SPLIT (s); + xaccTransAppendSplit (trans, s); + xaccAccountInsertSplit (split->acc, s); + } } -#endif - } } else { @@ -537,6 +523,7 @@ xaccTransBeginEdit (Transaction *trans) assert (trans); trans->open = 1; xaccOpenLog (); + xaccTransWriteLog (trans, 'B'); } void @@ -574,7 +561,35 @@ xaccTransCommitEdit (Transaction *trans) } trans->open = 0; - xaccTransWriteLog (trans); + xaccTransWriteLog (trans, 'C'); +} + +/********************************************************************\ +\********************************************************************/ + +void +xaccTransDestroy (Transaction *trans) +{ + int i; + Split *split; + Account *acc; + + if (!trans) return; + CHECK_OPEN (trans); + xaccTransWriteLog (trans, 'D'); + + i=0; + split = trans->splits[i]; + while (split) { + MARK_SPLIT (split); + acc = split ->acc; + xaccAccountRemoveSplit (acc, split); + xaccAccountRecomputeBalance (acc); + i++; + split = trans->splits[i]; + } + + xaccFreeTransaction (trans); } /********************************************************************\ @@ -619,8 +634,11 @@ xaccTransAppendSplit (Transaction *trans, Split *split) } /********************************************************************\ + * TransRemoveSplit is an engine private function and does not/should + * not cause any rebalancing to occur. \********************************************************************/ + void xaccTransRemoveSplit (Transaction *trans, Split *split) { @@ -895,30 +913,32 @@ xaccTransSetDescription (Transaction *trans, const char *desc) #define SET_TRANS_FIELD(trans,field,value) \ { \ + char * tmp; \ if (!trans) return; \ CHECK_OPEN (trans); \ \ /* the engine *must* always be internally consistent */ \ assert (trans->splits); \ + assert (trans->splits[0]); \ \ + /* there must be two splits if value of one non-zero */ \ if (force_double_entry) { \ - assert (trans->splits[0]); \ - assert (trans->splits[1]); \ + if (! (DEQ (0.0, trans->splits[0]->damount))) { \ + assert (trans->splits[1]); \ + } \ } \ \ - if (0x0 != trans->splits[0]) { \ - char * tmp = strdup (value); \ - free (trans->splits[0]->field); \ - trans->splits[0]->field = tmp; \ - MARK_SPLIT (trans->splits[0]); \ - \ - /* if there are just two splits, then keep them in sync. */\ - if (0x0 != trans->splits[1]) { \ - if (0x0 == trans->splits[2]) { \ - free (trans->splits[1]->field); \ - trans->splits[1]->field = strdup (tmp); \ - MARK_SPLIT (trans->splits[1]); \ - } \ + tmp = strdup (value); \ + free (trans->splits[0]->field); \ + trans->splits[0]->field = tmp; \ + MARK_SPLIT (trans->splits[0]); \ + \ + /* If there are just two splits, then keep them in sync. */ \ + if (0x0 != trans->splits[1]) { \ + if (0x0 == trans->splits[2]) { \ + free (trans->splits[1]->field); \ + trans->splits[1]->field = strdup (tmp); \ + MARK_SPLIT (trans->splits[1]); \ } \ } \ } @@ -941,6 +961,9 @@ xaccTransSetAction (Transaction *trans, const char *actn) Split * xaccTransGetSplit (Transaction *trans, int i) { + if (!trans) return NULL; + if (0 > i) return NULL; + /* hack alert - should check if i > sizeof array */ if (trans->splits) { return (trans->splits[i]); } @@ -950,18 +973,21 @@ xaccTransGetSplit (Transaction *trans, int i) char * xaccTransGetNum (Transaction *trans) { + if (!trans) return NULL; return (trans->num); } char * xaccTransGetDescription (Transaction *trans) { + if (!trans) return NULL; return (trans->description); } time_t xaccTransGetDate (Transaction *trans) { + if (!trans) return 0; return (trans->date_posted.tv_sec); } @@ -977,7 +1003,9 @@ xaccTransCountSplits (Transaction *trans) void xaccSplitSetMemo (Split *split, const char *memo) { - char * tmp = strdup (memo); + char * tmp; + if (!split) return; + tmp = strdup (memo); if (split->memo) free (split->memo); split->memo = tmp; MARK_SPLIT (split); @@ -986,7 +1014,9 @@ xaccSplitSetMemo (Split *split, const char *memo) void xaccSplitSetAction (Split *split, const char *actn) { - char * tmp = strdup (actn); + char * tmp; + if (!split) return; + tmp = strdup (actn); if (split->action) free (split->action); split->action = tmp; MARK_SPLIT (split); @@ -995,6 +1025,7 @@ xaccSplitSetAction (Split *split, const char *actn) void xaccSplitSetReconcile (Split *split, char recn) { + if (!split) return; split->reconciled = recn; MARK_SPLIT (split); xaccAccountRecomputeBalance (split->acc); diff --git a/src/engine/TransactionP.h b/src/engine/TransactionP.h index 7715812642..c9e9581c99 100644 --- a/src/engine/TransactionP.h +++ b/src/engine/TransactionP.h @@ -131,7 +131,7 @@ void xaccFreeSplit (Split *); /* frees memory */ /* The xaccTransRemoveSplit() routine will remove the indicated * split from the transaction. It will *NOT* otherwise - * readjust balances, modify accounts, etc. + * re-adjust balances, modify accounts, etc. */ void xaccTransRemoveSplit (Transaction*, Split *);