Re: dbGetPdbAddrFromLink dropped from 3.15 again

[Andrew Johnson <anj@anl.gov>]

On 08/06/2018 11:15 AM, Michael Davidsaver wrote:
> On 08/04/2018 08:43 AM, Johnson, Andrew N. wrote:
>> I would be wary of just handing out the dbAddr* pointer though,
>> since the link could get changed without any notice, so I would
>> like to add a little more enforcement. Some link types could even
>> use this to provide access to their raw data buffers, so I’m
>> wondering about an API like doLocked() where the caller gives us a
>> function pointer which we call back with a dbAddr*.
> 
> My thinking has been something like:
> 
>> int dbGetPdbAddrFromLink(DBLINK *plink, DBADDR *pout);
> 
> which would be used like
> 
>> DBLINK *plink = ...;
>> DBADDR addr;
>> dbScanLock(plink->precord);
>> if(dbGetPdbAddrFromLink(plink, &addr)==0) {
>>    // do something with addr
> 
> Though this would look different in the context of an aSub function,
> the point is that this avoids returning a pointer to the internal
> struct dbAddr by making a copy.  This copy remains valid in
> perpetuity, although the assurance that both records are covered by
> the same lock expires as soon as the source record is unlocked.

I don't believe it's legal to call any current dbLink.h function unless
you're already holding the lock for the record that owns that link, so
IMO your dbScanLock() should be superfluous.

Copying a dbAddr also copies its pfield member pointer. If the target
field of the link is using double-buffering that pointer isn't going to
be valid after the next buffer-switch, but your API doesn't indicate that.

That's why I'd prefer to have the user provide a callback function; it
should be obvious that the information provided is only valid for the
duration of the call (we can also avoid making a copy of the dbAddr):

typedef long (*dbLinkDbAddrCallback)(const dbAddr *paddr, void *priv);
long dbLinkDoDbAddr(struct link *plink, dbLinkDbAddrCallback rtn,
        void *priv);

This would call a new lset routine which only DB links would implement;
other link types would just return an error (unless they are willing to
give out access to their local buffer for the link data, in which case
they could create a dbAddr for that buffer).

In dbDbLink.c the implementation is just:

static long doDbAddr(struct link *plink, dbLinkDbAddrCallback rtn,
        void *priv)
{
    DBADDR *paddr = (DBADDR *) plink->value.pv_link.pvt;

    return rtn(paddr, priv);
}


Till, nothing more is likely to happen on this topic without your
feedback, would this API be sufficient to implement what you need?

- Andrew

-- 
Arguing for surveillance because you have nothing to hide is no
different than making the claim, "I don't care about freedom of
speech because I have nothing to say." -- Edward Snowdon