Re: cbdata

Alex Rousskov wrote:

> My understanding is that it may become invalid. Thus, the callback
> must call cbdataValid() before accessing the pointer. I think Henrik
> is just trying to avoid calling the callback if the data is already
> invalid (and decrement reference count at the same time).

The callback does not need to call cbdataValid() before accessing the
pointer because it per definition owns the object, but the one calling
the callback MUST before making the callback.

The owner of the object only uses cbdataCreate() and cbdataDestroy(). It
does not need to care about references as it owns the object.

If it no longer wants the object, cbdataDestroy() is called, causing any
other references to become invalid.

When the owner needs to call another module that returns with a
callback, then the cbdata object is passed as the callback argument. The
other module MUST then create a reference and store this reference in
it's internal data next to the callback pointer.

When the other module is ready to signal back to the caller via the
callback, it must first verify the validity of the reference and only
perform the callback if the data is valid.

   if ((void *cbdata = cbdataAbandon(mystate->callback_data) != NULL)

line both verifies that the data is valid and decreases the reference
count. After this mystate->callback_data no longer refers to the object,
and cbdata has a temporary reference to the object IFF the object is
valid.

If the object is invalid then cbdata is NULL.

Note: There is one small difference between the cbdataAbandon return
value approach and cbdataValid() checks. cbdataValid() is always true
for NULL pointers. However, the cases where NULL data handles are used
is very few, close to extinct, and I doubt it is a practice to
recommend.

One sligthly longer example

/***** caller ******/

typedef struct {
   ...
} callerState;
CBDATA_TYPE(callerState);

void callerInit(void)
{
    callerState *state = cbdataCreate(callerState);
    .....
    /* now needing to make a call to foo */
    fooStart(..., callerFooDone, state);
}

void callerFooDone(..., void *cbdata)
{
    callerState *state = cbdata;
    /* continue, process the result of foo */
    ....
}

/* This is called if caller is aborted */
void callerAbort(...)
{
    ...
    cbdataDestroy(state);
}

/**** foo ****/

struct mystate {
    ...
    void (*callback)(..., void *data);
    void *callback_data;
}

void fooStart(..., callback, callback_data)
{
    ...
    mystate->callback = callback;
    mystate->callback_data = cbdataUse(callback_data);
    ...
}

void fooComplete(...)
{
    ...
    if ((void *cbdata = cbdataAbandon(mystate->callback_data)) != NULL)
        mystate->callback(..., cbdata);
}

As said previously, I think the names cbdataUse/Abandon not the proper
words here... "foo" is not using or abandoning the object, it is only
storing a reference to the object to be passed back to the caller later.
In most cases "foo" does not use of the object, or even have knowledge
on how to use it. The object is internal to the caller.

In the current code there are a few exceptions where cbdata is used for
other constructs than callbacks, but these are not the main point of
cbdata.

Regarding the other issue brought up earlier today about installing
destroy/invalidation hooks. Personally I think these make much sense, as
"foo" may want to abort it's operations if "caller" aborts. However, it
is unclear how to best implement such a design, but from what it looks
doing so requires the introduction of a "reference" object to keep track
of the reference and it's hooks. Probably not a bad idea, but does not
play well with the non-callback uses of cbdata actually caring about the
referenced data type...

--
Henrik