[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Convert Notify/Log/Prompt callbacks to structs

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2003-07-15 19:16:07 CEST

"D.J. Heap" <dj@shadyvale.net> writes:

> 1. Change it to build a struct and pass it in place. Probably
> simplest, but would take the most coding to convert and, IMO, is
> something of a pain to code up new notify callers (gotta build a
> struct, init all fields, etc). Also has low overhead. A drawback
> (IMO) is that new fields added to the notify struct may not get set
> properly in all places if careful attention is not paid to all callers
> when they are added -- of course, old caller code may not care to set
> new fields anyway (beyond a zero value).
> For example, instead of:
> if (baton->notify_func)
> (*baton->notify_func) (...)
> It would be something like:
> if (baton->notify_func)
> {
> struct svn_wc_notify_struct_t n; // or put this in a pool?

Don't use a pool if you can use the stack.

> memset(&n, 0, sizeof(n));

I dislike like unnecessary initialisation, I feel it hides bugs.

> n.baton = notify_baton;
> n.path = path;
> ...
> (*baton->notify_func) (&n);
> }

    if (baton->notify_func)
         struct svn_wc_notify_struct_t n;
         some_new_function (&n, the, original, args);

> 2. Use a wrapper macro or function to build the struct and call the
> callback. Less typing for me to convert the current caller code, and
> IMO, easier for new callers to use, and when new fields are added the
> old caller code will get an error or warning about not having all the
> parameters. A macro would introduce less overhead than a wrapper
> function.

The "overhead" of a function call is almost certainly trivial,
particularly in the case when the callback produces some output.

> Something like:
> SVN_WC_NOTIFY(baton->notify_func, notify_baton, path, ...)

I don't like the idea of using a macro here.

> 3. Leave well enough alone. Very easy to do. Only real drawback is
> some pain for clients when new fields are passed in the callbacks.

I'm not sure I understand the above.

The notify callback has several "actions", and some of the parameters
are not relevant for all the actions. This is one of the reasons I
dislike the memset initialisation, why bother setting a field that is
not used? Perhaps it would be clearer to use a union of structs, with
one struct for each action or group of actions, so that it is easy to
determine which parameters are relevant for each action.

Philip Martin
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jul 15 19:17:03 2003

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.