[DGD] A bug and a question about reference counting and DGD internals

[Foo the Xyzzy Avenger]

Hi list,

First, the bug:

I've been modding the driver (1.2.124) recently via the kfun extension
interface.  So far it's been a fairly painless experience.  I did find
a bug, however: the macro definition of  DGD_OBJECT_DATASPACE in
dgd_ext.h on line 124 is defined as o_data((o)), which ought to be
o_dataspace((o)), as the former doesn't exist and the latter does what
the macro is intended to do.

Now to the meat of my question: what is the lowdown on reference
counting for arrays and strings?  When should I increment a reference
in my own kfuns?  And why are there no DGD_* macros provided to do
this?

Basically, I wrote an extension kfun that returned an array of type
mixed **.  The C code constructs this array iteratively, but I quickly
discovered that during cleanup in endthread() the driver crashed (or
rather, aborted) due to a bad pointer passed to m_free().  Not being
intimately familiar with the driver's memory structure, I nonetheless
suspected, especially after a session with gdb, that the subarrays
contained in the array returned by the kfun having a refcount of 0 had
something to do with it.  The DGD_RETVAL_* macros do increment the
refcount for arrays and strings, so returning a single-dimensional
array never causes problems, but the minute you try to put arrays into
that array, you're up the proverbial creek.  A call to arr_ref() fixed
this problem immediately; however, arr_ref(), despite being ubiquitous
in the code, is not part of the published "kfun extension standard" --
perhaps a DGD_ARRAY_REF or similar macro should be defined?

This now leads me to strings.  I am a little bit confused about
strings, and when their references should be incremented.  When I
return a string from a function (for example, the lower_case() example
kfun included in doc/Extensions) the DGD_RETVAL_STR macro does
increment the refcount, so it seems as though strings I return (say,
as elements of an array) should also have their references
incremented.  However, nothing obviously bad happens when I don't, so
I'm not totally sure.

Consider another (example) kfun that permutes the elements of an array
in a random fashion, and returns it.  Each of these strings (which are
not modified by the kfun, and are simply placed in an array as-is)
should have its refcount incremented, right?  That much seems obvious.

Basically what confuses me is that DGD_STRING_PUTVAL and
DGD_RETVAL_STR in fact differ only in that the latter increments the
refcount, whereas the former does not.  Their definitions in terms of
PUT_STRVAL and PUT_STRVAL_NOREF make this clear, but their names make
it seem as though DGD_RETVAL_* should only be used when manipulating
the return value passed (by reference) to the kfun; indeed, that it
was intended to do only this is especially evident given that
DGD_RETVAL operates on value pointers, whereas DGD_STRING_PUTVAL and
friends operate on value types.  This seems fairly confusing for
something that ought to be straightforward.

Another minor thing: why are the DGD_*_T types macro #defines?  This
causes a great deal of headache with pointers.  For example,
DGD_STRING_T is #define'd as string *; therefore, the following
declaration, which seems like it ought to work:

DGD_STRING_T str1, str2;

actually expands to

string *str1, str2;      /* str1 and str2 are not the same type! */

This is exactly the reason that C has typedefs, and their use in the
rest of the driver is ubiquitous; why are we using the preprocessor,
which everyone knows is an ugly hack on the best of days, to do
something that the C language actually has fairly elegant machinery in
place to handle?

Overall, though, I've been impressed with the cleanliness and
portability of DGD, and how generally bug-free it is.  Good show,
Dworkin.

I look forward to any clarification on reference counting you guys can
give me.  I also humbly suggest that arr_ref, str_ref and friends be
given DGD_* extension macros or, alternatively, accepted as standard
usage for kfun extensions, and that an explanation of when to use them
be added to doc/Extensions.  In fact, I'll even submit the
documentation changes myself, once I get a better a handle on how it
should be done.

Thanks in advance!