Revert Linas' broken changes to qof.

Derek Atkins warlord at MIT.EDU
Mon Apr 19 10:17:11 EDT 2004


linas at linas.org (Linas Vepstas) writes:

> Uhh, except for QOF_TYPE_DOUBLE which bombs because a double
> doesn't fit into the default 32-bit return value.  So we already
> have a (far more serious) problem there, which we haven't seen 
> only because I don't think we use QOF_TYPE_DOUBLE anywhere.

I really would like to deprecate DOUBLE if at all possible.  (It may
not be possible).  Where in the code are DOUBLE's used?  _can_ we
"deprecate" them (and by deprecate I mean "rip them out")?

> Point being AccessFunc is already broke in a more serious way.

I'm not sure this is more serious.  First, the function is cast to a
(double (*)(gpointer)) before it's used -- or at least it USED to do
that before you changed it to cast to (double
(*)(gpointer,QofParam*)).  The actual function being called is
declared to return a double.  So in the end you've got the right
API/ABI between the caller and callee (at least with my original
code).

Perhaps I did a disservice by overloading getters that return other
objects with getters that return parameter data?

Could you perhaps explain why you need to pass the QofParam into the
getter function?  Are you trying to overload getters so you only need
to implement one function capable of returning multiple parameters?
If so, then I've got a suggestion:

Change the Object definition to add an 'int param_def_internal'
member, an "internal parameter definition".  The particular object
module can define an enum internally for all its parameters and then
in the 'common getter' can just switch() on the param_def_internal
member of the QofParam to figure out which parameter to return.
Obviously you'll need a different "common getter" per data type
(string, numeric, boolean, date, etc).  But that's definitely fewer
functions to implement than one-per-parameter, right?

It DOES mean you need to change EVERY object definition in GnuCash and
no longer directly reference the xaccFooGetBar() functions, but you
already seem inclined to this change.  The compiler might even help
you if you add the internal parameter definition member to QofParam
(unlike changing the function API, which the compiler doesn't catch
due to all the casts everywhere).

> I'm aware of what I did ... 

Yes, but other people on this list might not be aware.  This was for
them, not just for you.  I don't expect everyone to be monitoring
gnucash-changes like I do :)

> I'm pretty sure it'll work on all systems and compilers.  
> The calling convention is determined by the ABI.  I don't
> beleive that there are any ABI's that exchange the order of 
> the args.  And clobber semantics are also defined as part of 
> the ABI. Compilers must adhere to the ABI.  So we are safe 
> just about everywhere.

See, I'm not convinced it will work on all compilers on all ABIs.
There may be some ABIs that push the args in forward order and some
that push them on in reverse order.  So I'm not at all convinced we're
safe "just about everywhere".

>> We're trying to write portable code, aren't we?
>
> Yes.  I guess that means that the only way to make you happy 
> would be to make *all* of the needed changes, and check it all in
> concurrently?  I'll do that.

Well, checking in non-working code that doesn't build is generally a
no-no on HEAD (or any other major branch).  You're always welcome to
create your own development branch if you want to commit changes over
time to get some new features working, and then merge into HEAD when
you're done.  I'm certainly considering this approach when I go heads
down into the move from XML->SQL as the default "file" format.

Bugs notwithstanding, yes, I would prefer if you checked in all your
code at once, after you got it all working.  If you want to make a few
sets of commits over a period of a few minutes that's fine, too -- so
long as it's really a few minutes and not a few hours or days.

Thanks for your consideration,

-derek
-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       warlord at MIT.EDU                        PGP key available


More information about the gnucash-devel mailing list