Macros that affect program flow. (was Re: temporary absence)
Neil Williams
linux at codehelp.co.uk
Sun Feb 26 04:36:17 EST 2006
On Sunday 26 February 2006 3:17 am, you wrote:
> > Apart from that and the changes committed to trunk, does anyone have any
> > more code out-of-tree that should go into QOF 0.6.2?
>
> Yes. I've got some changes related to QOF_COMMIT_EDIT_PART2.
OK, I've got r13390 and r13391 (guid.c). PROBLEM:
The macros in qof-be-utils.h are awkward because they (all) change programme
flow. We can't redefine QOF_BEGIN_EDIT as a call to qof_begin_edit because
functions that use QOF_BEGIN_EDIT rely on it calling return if the instance
is NULL or edit_level < 1. I don't like macros that work like that and I
would like to deprecate each of these macros and force the use of the
functions in libqof2.
We cannot redefine QOF_COMMIT_EDIT_PART2 because the last call in the macro is
'return':
- if ((inst)->do_free) { \
- (on_free)(inst); \
- return; \
- }
Your code simply returns from the qof_commit_edit_part2 function, it does NOT
return from the function that called the original macro. To do that, existing
code would have to check the return value of the function that is behind the
macro. Not impossible, but not required by the current API either. That's an
API change.
I got bitten by this problem when I first tried to use a function in place of
QOF_BEGIN_EDIT.
IF the macro is NOT followed by any further code in the calling function, this
would be OK but there is nothing to say that this is the case (or has to be
the case). The current API (rightly or wrongly) allows programmes to call
QOF_COMMIT_EDIT_PART2 and then continue processing the instance. Such code is
assuming that if the instance was freed as part of the commit that the macro
will have returned OUT of the function itself. A function replacing
QOF_COMMIT_EDIT_PART2 will still free the instance but now processing
continues into the rest of the function which is not expecting a freed
instance to be possible. Even having a LEAVE macro that references the now
freed instance would cause a seg fault.
Derek - I would v.v.v.much like to deprecate all the macros in qof-be-utils.h
precisely because of these problems. They cause no end of confusion and
hidden assumptions. I know you wanted these as macros but I think the API is
too confusing. IMHO, the pros of the macros do not outweigh their problems.
The functions are easier to use - particularly because they can be used in
functions that return a value. To me, macros should not determine whether or
not I can return a value from an internal function.
In fact, once the macros are deprecated (i.e. moved into deprecated.h and
tagged in doxygen), qof-be-utils.h can be removed and the function
declarations made part of qofbackend.h (where I think new users of libqof
would expect to find them).
Chris: We need to replace the macro and have the function as an alternative,
*not* a replacement. It matters not whether gnucash has a problem with the
function, it matters that the API is changed by the change in programme flow
generated within the old macro but not implementable in the function version.
> I have
> verified that this macro is not used outside of GnuCash.
That's not the whole story for an API issue - a macro in an API must retain
it's arguments and behaviour, especially it's use of 'return;'.
i.e. code that uses the old macro must still function the same way. The macro
is still part of the API (until libqof2). We need the macro replaced, for
now. I don't like it, but that's the way it is until libqof2.
If there is agreement to deprecate all these macros, then the functions will
replace all the macros in libqof2. Until then, libqof1 requires the old macro
to be in place.
:-(
--
Neil Williams
=============
http://www.data-freedom.org/
http://www.nosoftwarepatents.com/
http://www.linux.codehelp.co.uk/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.gnucash.org/pipermail/gnucash-devel/attachments/20060226/c64f744b/attachment-0001.bin
More information about the gnucash-devel
mailing list