Problems with qof_begin_edit and qof_commit_edit

[Chris Shoemaker]

On Sun, Oct 02, 2005 at 12:00:06PM -0400, David Hampton wrote:
> I have some concerns with these functions that have been committed to
> the code over the last several months.  In some places they replace
> similarly named macros, in others they are newly added to existing code.
> My concerns are:
> 
> 1) These function do not have the exact characteristics of the macros
> they replace.  Specifically, a return call from the macro exits the
> calling function.  A return in one of these functions only exits the
> function, not the caller.  The caller then executes code that was not
> executed prior to the macro->function change.  This behavior is what
> caused the crashes that both Didier and I saw.  This is clearly broken
> and must be fixed.
> 
> 2) Many places have had a qof_begin_edit()/qof_commit_edit() pair of
> function calls wrapped around existing code.  In this case the control
> flow through the pre-exiting code is the same as before.  I.E. It always
> occurs.  My concern is whether or not this is the proper control flow.
> If qof_begin_edit() bails out because the object is already being
> edited, should any other code be executed or should the calling function
> exit immediately?  I believe the calling function should exit
> immediately (i.e. don't modify an object that is being modified
> elsewhere).
> 
> Fixing either of these problems requires a (backward compatible) change
> to the current QOF API.  These two functions should return a boolean
> value that indicate whether they executed successfully or bailed out.
> This will provide information back to the calling function that it
> originally had when a macro definition was used.  The calling function
> can then exit immediately if the qof_xxx_edit function bails.  It seems
> to me that all callers should test the return value to see if
> qof_begin_edit bailed.  Calls to qof_commit_edit should test the return
> value if they aren't the last line of the function.
> 
> Thoughts, anyone?

Few things are as abusive as macros that return.  Actually, macros
that affect control flow at all are EVIL.  I would gladly remove all
such macros.  That said, ATM, I'm especially sensitive to making even
good, widespread changes to G2 that don't move toward a release. (as
you can probably tell from other thread.)

I think we can, and should say, "yeah, this is good code, yeah these
are good changes - let's queue these up for right after g2 first g2
release."  Is anyone actually doing this besides me?

-chris