Register code question

Charles Day cedayiv at gmail.com
Thu Oct 16 23:13:06 EDT 2008


On Thu, Oct 16, 2008 at 4:27 AM, Chris Shoemaker <c.shoemaker at cox.net>wrote:

> On Wed, Oct 15, 2008 at 08:41:58PM -0700, Charles Day wrote:
> > I am debugging the register code and trying to figure out why bogus
> message
> > boxes can appear, and also why crashes occur in certain cases. I believe
> > that I've figured it out the sources of these problems, but I have a
> > background question to ask before proceeding with a fix.
> >
> > These problems seem to stem from the way that the "pending_trans_guid" is
> > used when entering a brand new transaction. Can anyone explain the
> intended
> > use of pending_trans_guid where new transactions are concerned?
> >
> > I came across some comments in split-register-load.c which appear to shed
> > some light. When the bottom line of the register is created (where the
> user
> > would start entering a new transaction), a new transaction with a single
> > blank split is tied to it but not marked pending:
> >     /* We don't want to commit this transaction yet, because the split
> >        doesn't even belong to an account yet.  But, we don't want to
> >        set this transaction as the pending transaction either, because
> >        we want to pretend that it hasn't been changed.  We depend on
> >        some other code (somewhere) to commit this transaction if we
> >        really edit it, even though it's not marked as the pending
> >        transaction. */
>
> [disclaimer: This is totally from my poor memory.  Your best bet for
> getting a deep understanding the register code is thoroughly studying
> the code and history, and single stepping in gdb.]
>
> pending_trans_guid is how the register keeps track of an open, edited
> transaction.  From the user's perspective, navigating to the blank
> transaction is not an "editing" operation (especially since it happens
> automatically when the register opens) and should therefore not dirty
> the book.
>
> >
> > So it seems the intention is to never mark a brand-new transaction as the
> > pending transaction.
>
> Not exactly, the intention is to not mark a brand-new transaction as
> pending until it is actually edited.
>
> > Yet a few routines DO set the brand-new transaction as
> > pending, such as the autocompletion routine
> > gnc_split_register_auto_completion() in split-register-control.c, and a
> > couple of other routines that perform split deletion.
>
> That seems correct.
>
> > So does anyone know why some forms of editing (autocompletion, deleting a
> > split) cause the brand-new transaction to be marked as pending, while all
> > others, including the typical practice of simply typing a transaction in,
> do
> > not?
> >
> > What is the correct practice?
>
> At a higher level, the ideal behaviors are:
>
>  a) Opening and navigating in a register doesn't dirty the book.
>  b) Editing a transaction doesn't dirty the book until it is committed.
>  c) Closing a register while on an edited, uncommitted transaction will
>     raise the modal commit/rollback dialog.
>  d) Autocompletion counts as editing the transaction.
>
> Hope that helps,
>

Thank you, that certainly helps. I've just committed r17628 which prevents
brand-new transactions from being mistaken for existing transactions that
are being edited in another register. When combined with r17623, this fixes
bugs 393383 and 426111. These two patches are pretty small. If you have a
minute to give them a quick proofread, at least for a conceptual
double-check, I'd be interested in any comments. To me this fixes some
fairly obvious bugs (to a fresh pair of eyes, at least) although at this
point perhaps I just know enough to be dangerous. :)


> -chris
>
> p.s. Please be especially careful with register code.  You probably
> know this, but over the years, that code has seen a pattern of subtle
> regressions that have taken months or years to fix (if ever?), and
> those bugs have caused data corruption.  It is _extremely_ easy to
> break in subtle ways.
>

Thanks, I'm trying to be careful, ask questions, and make minimal changes.

Cheers,
Charles


More information about the gnucash-devel mailing list