On Fri, May 16, 2008 at 10:59 AM, Tom Lane <[EMAIL PROTECTED]> wrote:
> "Merlin Moncure" <[EMAIL PROTECTED]> writes:
>>> typedef void *(*PGobjectEventProc)(PGobjectEventId evtId, ...);
>>> int PQregisterObjectEventProc(PGconn*, PGobjectEventProc);
>>> void *PQeventData(PGconn *, PGobjectEventProc);
>>> void *PQresultEventData(PGresult *, PGobjectEventProc);
>> This provides what we need...a key to lookup the hook data without
>> using a string. Also, it reduces the number of exports (it's a little
>> easier for us, while not essential, to not have to register each
>> callback individually). Also, this AFAICT does not have any ABI
>> issues (no struct), and adds less exports which is nice. We don't
>> have to 'look up' the data inside the callbacks..it's properly passed
>> through as an argument. While vararg callbacks may be considered
>> unsafe in some scenarios, I think it's a good fit here.
> I don't think varargs callbacks are a good idea at all. Please adjust
> this to something that doesn't do that. Also, haven't you forgotten
> the passthrough void *?
We didn't...one of the functions (the init) doesn't need it so we
didn't expose it to the fixed arguments...we would just va_arg it off
in the other cases. Regardless, your comments below make that moot:
> If you really want only one callback function, perhaps what would work
> typedef void (*PGeventProc) (PGeventId eventId, const void *eventInfo,
> void *passthrough);
> int PQregisterEventProc(PGconn *conn, PGeventProc proc, void *passthrough);
> where eventInfo will point to one of several exported struct types
> depending on the value of eventId. With this approach, you can add
> more fields to the end of any one such struct type without creating
> ABI issues. I have little confidence in being able to make similar
> changes in a varargs environment.
this is fine.
> Also, even if varargs are safe they'd be notationally unpleasant
> in the extreme. varargs are just a PITA to work with --- you'd have
> to do all the decoding in the first-level hook routine, even for
> items you weren't going to use. With something like the above
> all you need is a switch() and some pointer casts.
Switch, plus struct (basically a union) will do the trick nicely. Can
it be a formal union, or is it better as a void*?
The main issue was how what we called the 'hook data' was passed back
and forth. We'll get a patch up.
Sent via pgsql-patches mailing list (firstname.lastname@example.org)
To make changes to your subscription: