Greg Stein <gstein@lyra.org> writes:
> On Thu, Nov 30, 2000 at 07:25:55PM -0000, sussman@tigris.org wrote:
> >...
> > +typedef struct svn_ra_plugin_t
> > +{
> > + svn_string_t *name; /* The name of the ra library,
> > + e.g. "ra_dav" or "ra_local" */
> > +
> > + svn_string_t *description; /* Short documentation string */
>
> These should be "const char *". Nobody is going to do any string
> manipulation with them (and if they do, they can wrap them into a string at
> that point). By using the simpler type, we can also create a static
> structure like so:
>
> static const svn_ra_plugin_t plugin = {
> "ra_greg",
> "some description goes here",
> my_ra_open,
> ...
> };
Gotcha. Good suggestion, I agree.
>
> > + apr_dso_handle_t *my_dso; /* handle on the actual library loaded */
>
> By putting this in here, we toss out the option of using "const" in the
> above declaration.
>
> Within libsvn_client, you can have a hash that maps NAME to { DSO,
> PLUGIN-PTR }.
>
Oh! Sure.
> >...
> > +typedef struct svn_ra_init_params
> > +{
> > + apr_pool_t *pool; /* where to allocate new plugin */
> > + apr_array_header_t *client_plugins; /* client's private array of
> > + plugins */
>
> I think this would be a hash table.
Then let's scratch this whole structure. Why do we even need
svn_ra_register_plugin? I mean, why not have each ra library export
svn_ra_FOO_init (svn_ra_plugin_t **new_plugin)
The client then gets a handle to the static const plugin_t within the
library, and adds it to its hash of loaded plugins. Isn't this simpler?
>
>
> >...
> > + When called by libsvn_client, this routine must:
> > +
> > + * allocate a new ra_plugin object from params->pool
>
> I believe it is possible to use a "static const" structure rather than an
> allocation (and the resulting, add'l complexity).
>
Right, this agrees with what you said above. Makes sense.
> > +/* Greg, please update ra_dav to use the declarations above, not
> > + below! I'm leaving these here so that we can still compile. */
>
> I'll start tweaking ra_dav to this latest rev.
>
Of course, I'll be making all these interface changes now too.
Gee, I'm so agreeable tonight. Thanks for the feedback. :)
Received on Sat Oct 21 14:36:15 2006