You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Ian Holsman <in...@webperf.org> on 2001/08/21 01:50:25 UTC
Re: [proposal] DBM - allow multiple DBM's of differnt types at the
same time.
On Mon, 2001-08-20 at 13:07, Sterling Hughes wrote:
> > >
> > >Well, I think we should choose one, and just follow that one model. I personally
> > >would rather use an argument to specify the type. We can't do that with buckets,
> > >because that would require function overloading. SMS should move to just
> > >a simple argument IMHO though.
> > >
> > the problem passing a option to the dbm open is twofold.
> >
> > 1. If I want to add another dbm (say NuSQL's genesis or some comerical
> > package) I would then need to
> > modify apr-util and add the flag. this means a branch and will present a
> > road block for me upgrading apr-util
> >
>
> Not sure I understand what you're saying? There are ways to allow
> the registering of module functionality, without modifying the main
> package. Just provide a function like:
>
> apr_dbm_register()
>
> Which registers a dbm internally, and then have a constant defined
> in a seperate file, and all is clear...
that would work,
but that wouldn't that mean there would be need to some init code
somewhere to call the registration function?
>
> > 2. I may have a dbm which needs extra parameters to open it. (say for
> > exampe cache size, or maybe it requires
> > a fixed key length defined) I couldn't do this simply
> >
>
> dbm_set_x() and check the flags, once they're all filled out (a
> va_args implementation could also be done...)
>
> Furthermore, the idea of an abstraction layer is too bring all these
> down to a lowest common denominator, at the cost of functionality
> sometimes; usually functions that take extra arguments, can be
> filled in with acceptable defaults. You'll run into the same
> problems in that some db's don't support certain features, and
> others do, the idea is to concentrate on the functionality that is
> necessary and shared between the different db's (wrap this in a
> large imho :).
>
true, thats why they all return the same thing (apr_dbm_t)
the open_DBM function was supposed to be called when the devloper wanted
to do something specific with this type of dbm. so in the berkley DB
case there could be a apr_dbm_db_getRaw() function which returns the DB*
so that developers are not constrained the set of functions we chose to
abstract.
(BTW I was planning on implmenting a shared-memory hash table, that I
posted ages ago, via this DBM interface, and it required a different set
of opening parameters (key/elem size, #elements) these parameters could
be passed via a db_set function call but it would look clunky
..Ian
so..
shall we put it to a vote?
apr_dbm_open_XXX
or
apr_dbm_open(XXX)
?
(I'm assuming that APR-Committers are happy that I go messing with dbm
in the first place ;-)
> -Sterling
>
Re: [proposal] DBM - allow multiple DBM's of differnt types at
the same time. -- V2
Posted by Ian Holsman <ia...@cnet.com>.
On Sun, 2001-08-26 at 01:52, Greg Stein wrote:
> On Sat, Aug 25, 2001 at 06:27:23PM -0700, Ian Holsman wrote:
> > does anyone have any objections to this?
> > all it is really doing is removing the MACRO definitions
> > and splitting up the apr_dbm into 4 files (a interface, and 3 implementations)
> > the interface has not changed at all.
>
> I'd still like to review it first, but haven't had the chance yet.
>
I think I'll backport the berkeleyDB patches to the exisitng code and
get that going in the mean time.
the code as is doesn't work on berkeley.
> -g
>
--
Ian Holsman IanH@cnet.com
Performance Measurement & Analysis
CNET Networks - (415) 364-8608
Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time. -- V2
Posted by Greg Stein <gs...@lyra.org>.
On Sat, Aug 25, 2001 at 06:27:23PM -0700, Ian Holsman wrote:
> does anyone have any objections to this?
> all it is really doing is removing the MACRO definitions
> and splitting up the apr_dbm into 4 files (a interface, and 3 implementations)
> the interface has not changed at all.
I'd still like to review it first, but haven't had the chance yet.
-g
--
Greg Stein, http://www.lyra.org/
Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time. -- V2
Posted by Ian Holsman <ia...@cnet.com>.
does anyone have any objections to this?
all it is really doing is removing the MACRO definitions
and splitting up the apr_dbm into 4 files (a interface, and 3 implementations)
the interface has not changed at all.
The next part (registering/multiple DBMs) can be done after this stuff goes through.
..Ian
Ian Holsman wrote:
> Changes since last time:
>
> * NO CHANGE IN APR_DBM.H (so no diffs ;))
> * added licence file to dbm_private and put it in
> private/apr_dbm_private.h
> * GDBM is now there
> * patch to apr_dbm.c.patch
> * vtables are now static constants. (as per buckets)
> * code style-cleanup to make it more apache. (might still be a stray
> tab)
>
> This version does NOT do:
> * apr_dbm_open(XXX,...)
> * apr_dbm_registration
> * allow for multiple DBMs.
>
> all it really does at the moment is split the file into 3 parts.
>
> All it requires for existing 'users' of the DBM is a re-link to the new
> library.
>
> IF this patch is Ok, then I'll get the multi-dbm's working (which means
> changing the configure, and adding the registeration/open(XXX))
>
> XXX_Usednames is not part of the hook, as you don't need a DBM open to
> call it. (and changing it to require it would break mod_dav, as it
> doesn't seem to have a open DBM when it calls the funciton)
>
> ??comments???
> Ian
>
>
Re: [proposal] DBM - allow multiple DBM's of differnt types at thesame time. -- V2
Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
It seems Cliff, Jeff and John Sterling have been kicking around the mod_auth_db
issue again, perhaps time to resurrect this patch and conversation?
Bill
----- Original Message -----
From: "Ian Holsman" <ia...@cnet.com>
To: "Greg Stein" <gs...@lyra.org>
Cc: <de...@apr.apache.org>
Sent: Friday, August 24, 2001 12:48 PM
Subject: Re: [proposal] DBM - allow multiple DBM's of differnt types at thesame time. -- V2
> Changes since last time:
>
> * NO CHANGE IN APR_DBM.H (so no diffs ;))
> * added licence file to dbm_private and put it in
> private/apr_dbm_private.h
> * GDBM is now there
> * patch to apr_dbm.c.patch
> * vtables are now static constants. (as per buckets)
> * code style-cleanup to make it more apache. (might still be a stray
> tab)
>
> This version does NOT do:
> * apr_dbm_open(XXX,...)
> * apr_dbm_registration
> * allow for multiple DBMs.
>
> all it really does at the moment is split the file into 3 parts.
>
> All it requires for existing 'users' of the DBM is a re-link to the new
> library.
>
> IF this patch is Ok, then I'll get the multi-dbm's working (which means
> changing the configure, and adding the registeration/open(XXX))
>
> XXX_Usednames is not part of the hook, as you don't need a DBM open to
> call it. (and changing it to require it would break mod_dav, as it
> doesn't seem to have a open DBM when it calls the funciton)
>
> ??comments???
> Ian
>
>
>
>
> On Wed, 2001-08-22 at 02:54, Greg Stein wrote:
> > -1 until you put that GDBM support back in. It isn't right to just drop it.
> >
> > On Mon, Aug 20, 2001 at 04:50:25PM -0700, Ian Holsman wrote:
> > > On Mon, 2001-08-20 at 13:07, Sterling Hughes wrote:
> > >...
> > > > Not sure I understand what you're saying? There are ways to allow
> > > > the registering of module functionality, without modifying the main
> > > > package. Just provide a function like:
> > > >
> > > > apr_dbm_register()
> > > >
> > > > Which registers a dbm internally, and then have a constant defined
> > > > in a seperate file, and all is clear...
> > > that would work,
> > > but that wouldn't that mean there would be need to some init code
> > > somewhere to call the registration function?
> >
> > Every time we load a module, we call registration functions. That is no big
> > deal.
> >
> > Your solution of apr_dbm_*_open() still requires a recompilation/relink. The
> > registration mechanism is completely open-ended.
> >
> > > > > 2. I may have a dbm which needs extra parameters to open it. (say for
> > > > > exampe cache size, or maybe it requires
> > > > > a fixed key length defined) I couldn't do this simply
> > > > >
> > > >
> > > > dbm_set_x() and check the flags, once they're all filled out (a
> > > > va_args implementation could also be done...)
> >
> > Not sure what you mean by dbm_set_x() here. You have to open the thing first
> > to get an apr_dbm_t. But then you're too late.
> >
> > The va_arg solution is a semi-reasonable solution, but that kills any chance
> > for type checking.
> >
> > > > Furthermore, the idea of an abstraction layer is too bring all these
> > > > down to a lowest common denominator, at the cost of functionality
> > > > sometimes; usually functions that take extra arguments, can be
> > > > filled in with acceptable defaults. You'll run into the same
> > > > problems in that some db's don't support certain features, and
> > > > others do, the idea is to concentrate on the functionality that is
> > > > necessary and shared between the different db's (wrap this in a
> > > > large imho :).
> >
> > Exactly. apr_dbm exposes DBM-style functionality. Simple key/value pairs.
> > That is it. The open functionality does not need flexibility. It has not
> > been demonstrated yet that we need more capability there.
> >
> > > true, thats why they all return the same thing (apr_dbm_t)
> > > the open_DBM function was supposed to be called when the devloper wanted
> > > to do something specific with this type of dbm. so in the berkley DB
> > > case there could be a apr_dbm_db_getRaw() function which returns the DB*
> > > so that developers are not constrained the set of functions we chose to
> > > abstract.
> >
> > No... we already have patterns for getting at underlying data types. See
> > apr_portable.h. Follow that pattern *IF* we are to expose those. IMO, we
> > should not do so.
> >
> > > (BTW I was planning on implmenting a shared-memory hash table, that I
> > > posted ages ago, via this DBM interface, and it required a different set
> > > of opening parameters (key/elem size, #elements) these parameters could
> > > be passed via a db_set function call but it would look clunky
> >
> > No... keys and values are arbitrary length, as specified by an apr_datum_t.
> > If your hash cannot handle that, then it cannot be used in the APR DBM
> > interface.
> >
> > >...
> > > so..
> > > shall we put it to a vote?
> > >
> > > apr_dbm_open_XXX
> > > or
> > > apr_dbm_open(XXX)
> >
> > Registration and the second form.
> >
> >
> > But your patch still has lots of work anyways. The GDBM functionality can't
> > just disappear. Sorry, but the DAV functionality uses apr_dbm and you'd
> > completely break access to the property files.
> >
> > Second, the vtable should not be part of the runtime apr_dbm_t structure.
> > You should have a pointer to a static const table, like we do with buckets.
> >
> > The formatting may have gone thru "indent" but it still sucks. Just a glance
> > over it shows some random gunk. For example, what the heck is up with the
> > formatting of apr_datum_t in apr_dbm.h? And all the spacing of the function
> > params in apr_dbm.h has been changed. There are extra blank lines at the end
> > of some functions (after returns), and some missing blank lines between
> > functions.
> >
> > We should also see a patch against apr_dbm.h, rather than the whole file. I
> > can't see if you've changed the public interface or not.
> >
> > dbm_private.h is missing a license and disclaimer. It also has a
> > non-standard #ifndef/#define/#endif pattern around it. We never start the
> > symbol with "_", and on all the new .h files you specified: toss the "1"
> > from the #define ... we don't do that either.
> >
> > Why is the "used names" functionality part of the per-db public interface?
> > That should be part of the hook stuff just like the rest.
> >
> > In any case... the per-db headers should go, in favor of the registration
> > mechanism. You would register each DB style with a name. You can then open a
> > DB style by name, or using the reserved "any". Note that we would define
> > some standard names:
> >
> > #define APR_DBM_USE_SDBM "sdbm"
> > #define APR_DBM_USE_GDBM "gdbm"
> > #define APR_DBM_USE_DB "db"
> > #define APR_DBM_USE_ANY "any"
> >
> > Also, the whole point of this exercise was to link in all of the variations.
> > That means the .c files should not have the #if APU_USE_*/#endif stuff
> > around them. The APU_USE_* symbols are only used to determine which DB to
> > use for the "any" case.
> >
> >
> > So... -1 for now. The patch still needs work before it can be applied.
> >
> > Cheers,
> > -g
> >
> --
> Ian Holsman IanH@cnet.com
> Performance Measurement & Analysis
> CNET Networks - (415) 364-8608
>
Re: [proposal] DBM - allow multiple DBM's of differnt types at the
same time. -- V2
Posted by Ian Holsman <ia...@cnet.com>.
Changes since last time:
* NO CHANGE IN APR_DBM.H (so no diffs ;))
* added licence file to dbm_private and put it in
private/apr_dbm_private.h
* GDBM is now there
* patch to apr_dbm.c.patch
* vtables are now static constants. (as per buckets)
* code style-cleanup to make it more apache. (might still be a stray
tab)
This version does NOT do:
* apr_dbm_open(XXX,...)
* apr_dbm_registration
* allow for multiple DBMs.
all it really does at the moment is split the file into 3 parts.
All it requires for existing 'users' of the DBM is a re-link to the new
library.
IF this patch is Ok, then I'll get the multi-dbm's working (which means
changing the configure, and adding the registeration/open(XXX))
XXX_Usednames is not part of the hook, as you don't need a DBM open to
call it. (and changing it to require it would break mod_dav, as it
doesn't seem to have a open DBM when it calls the funciton)
??comments???
Ian
On Wed, 2001-08-22 at 02:54, Greg Stein wrote:
> -1 until you put that GDBM support back in. It isn't right to just drop it.
>
> On Mon, Aug 20, 2001 at 04:50:25PM -0700, Ian Holsman wrote:
> > On Mon, 2001-08-20 at 13:07, Sterling Hughes wrote:
> >...
> > > Not sure I understand what you're saying? There are ways to allow
> > > the registering of module functionality, without modifying the main
> > > package. Just provide a function like:
> > >
> > > apr_dbm_register()
> > >
> > > Which registers a dbm internally, and then have a constant defined
> > > in a seperate file, and all is clear...
> > that would work,
> > but that wouldn't that mean there would be need to some init code
> > somewhere to call the registration function?
>
> Every time we load a module, we call registration functions. That is no big
> deal.
>
> Your solution of apr_dbm_*_open() still requires a recompilation/relink. The
> registration mechanism is completely open-ended.
>
> > > > 2. I may have a dbm which needs extra parameters to open it. (say for
> > > > exampe cache size, or maybe it requires
> > > > a fixed key length defined) I couldn't do this simply
> > > >
> > >
> > > dbm_set_x() and check the flags, once they're all filled out (a
> > > va_args implementation could also be done...)
>
> Not sure what you mean by dbm_set_x() here. You have to open the thing first
> to get an apr_dbm_t. But then you're too late.
>
> The va_arg solution is a semi-reasonable solution, but that kills any chance
> for type checking.
>
> > > Furthermore, the idea of an abstraction layer is too bring all these
> > > down to a lowest common denominator, at the cost of functionality
> > > sometimes; usually functions that take extra arguments, can be
> > > filled in with acceptable defaults. You'll run into the same
> > > problems in that some db's don't support certain features, and
> > > others do, the idea is to concentrate on the functionality that is
> > > necessary and shared between the different db's (wrap this in a
> > > large imho :).
>
> Exactly. apr_dbm exposes DBM-style functionality. Simple key/value pairs.
> That is it. The open functionality does not need flexibility. It has not
> been demonstrated yet that we need more capability there.
>
> > true, thats why they all return the same thing (apr_dbm_t)
> > the open_DBM function was supposed to be called when the devloper wanted
> > to do something specific with this type of dbm. so in the berkley DB
> > case there could be a apr_dbm_db_getRaw() function which returns the DB*
> > so that developers are not constrained the set of functions we chose to
> > abstract.
>
> No... we already have patterns for getting at underlying data types. See
> apr_portable.h. Follow that pattern *IF* we are to expose those. IMO, we
> should not do so.
>
> > (BTW I was planning on implmenting a shared-memory hash table, that I
> > posted ages ago, via this DBM interface, and it required a different set
> > of opening parameters (key/elem size, #elements) these parameters could
> > be passed via a db_set function call but it would look clunky
>
> No... keys and values are arbitrary length, as specified by an apr_datum_t.
> If your hash cannot handle that, then it cannot be used in the APR DBM
> interface.
>
> >...
> > so..
> > shall we put it to a vote?
> >
> > apr_dbm_open_XXX
> > or
> > apr_dbm_open(XXX)
>
> Registration and the second form.
>
>
> But your patch still has lots of work anyways. The GDBM functionality can't
> just disappear. Sorry, but the DAV functionality uses apr_dbm and you'd
> completely break access to the property files.
>
> Second, the vtable should not be part of the runtime apr_dbm_t structure.
> You should have a pointer to a static const table, like we do with buckets.
>
> The formatting may have gone thru "indent" but it still sucks. Just a glance
> over it shows some random gunk. For example, what the heck is up with the
> formatting of apr_datum_t in apr_dbm.h? And all the spacing of the function
> params in apr_dbm.h has been changed. There are extra blank lines at the end
> of some functions (after returns), and some missing blank lines between
> functions.
>
> We should also see a patch against apr_dbm.h, rather than the whole file. I
> can't see if you've changed the public interface or not.
>
> dbm_private.h is missing a license and disclaimer. It also has a
> non-standard #ifndef/#define/#endif pattern around it. We never start the
> symbol with "_", and on all the new .h files you specified: toss the "1"
> from the #define ... we don't do that either.
>
> Why is the "used names" functionality part of the per-db public interface?
> That should be part of the hook stuff just like the rest.
>
> In any case... the per-db headers should go, in favor of the registration
> mechanism. You would register each DB style with a name. You can then open a
> DB style by name, or using the reserved "any". Note that we would define
> some standard names:
>
> #define APR_DBM_USE_SDBM "sdbm"
> #define APR_DBM_USE_GDBM "gdbm"
> #define APR_DBM_USE_DB "db"
> #define APR_DBM_USE_ANY "any"
>
> Also, the whole point of this exercise was to link in all of the variations.
> That means the .c files should not have the #if APU_USE_*/#endif stuff
> around them. The APU_USE_* symbols are only used to determine which DB to
> use for the "any" case.
>
>
> So... -1 for now. The patch still needs work before it can be applied.
>
> Cheers,
> -g
>
--
Ian Holsman IanH@cnet.com
Performance Measurement & Analysis
CNET Networks - (415) 364-8608
Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time.
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Aug 22, 2001 at 08:00:17AM -0700, Ian Holsman wrote:
> On Wed, 2001-08-22 at 02:54, Greg Stein wrote:
> > -1 until you put that GDBM support back in. It isn't right to just drop it.
> >
> I was always intending to put GDBM back
> I developed what I had so far on NT, which didn't have GDBM
> I was going to work on GDBM on a linux box.
>
> is that cool?
When the patch has the GDBM stuff back in, then yes... But I had a number of
other concerns/questions, too. (did you see them further on in my note?)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: [proposal] DBM - allow multiple DBM's of differnt types at the
same time.
Posted by Ian Holsman <in...@webperf.org>.
On Wed, 2001-08-22 at 02:54, Greg Stein wrote:
> -1 until you put that GDBM support back in. It isn't right to just drop it.
>
I was always intending to put GDBM back
I developed what I had so far on NT, which didn't have GDBM
I was going to work on GDBM on a linux box.
is that cool?
Ian
> On Mon, Aug 20, 2001 at 04:50:25PM -0700, Ian Holsman wrote:
> > On Mon, 2001-08-20 at 13:07, Sterling Hughes wrote:
> >...
> > > Not sure I understand what you're saying? There are ways to allow
> > > the registering of module functionality, without modifying the main
> > > package. Just provide a function like:
> > >
> > > apr_dbm_register()
> > >
> > > Which registers a dbm internally, and then have a constant defined
> > > in a seperate file, and all is clear...
> > that would work,
> > but that wouldn't that mean there would be need to some init code
> > somewhere to call the registration function?
>
> Every time we load a module, we call registration functions. That is no big
> deal.
>
> Your solution of apr_dbm_*_open() still requires a recompilation/relink. The
> registration mechanism is completely open-ended.
>
> > > > 2. I may have a dbm which needs extra parameters to open it. (say for
> > > > exampe cache size, or maybe it requires
> > > > a fixed key length defined) I couldn't do this simply
> > > >
> > >
> > > dbm_set_x() and check the flags, once they're all filled out (a
> > > va_args implementation could also be done...)
>
> Not sure what you mean by dbm_set_x() here. You have to open the thing first
> to get an apr_dbm_t. But then you're too late.
>
> The va_arg solution is a semi-reasonable solution, but that kills any chance
> for type checking.
>
> > > Furthermore, the idea of an abstraction layer is too bring all these
> > > down to a lowest common denominator, at the cost of functionality
> > > sometimes; usually functions that take extra arguments, can be
> > > filled in with acceptable defaults. You'll run into the same
> > > problems in that some db's don't support certain features, and
> > > others do, the idea is to concentrate on the functionality that is
> > > necessary and shared between the different db's (wrap this in a
> > > large imho :).
>
> Exactly. apr_dbm exposes DBM-style functionality. Simple key/value pairs.
> That is it. The open functionality does not need flexibility. It has not
> been demonstrated yet that we need more capability there.
>
> > true, thats why they all return the same thing (apr_dbm_t)
> > the open_DBM function was supposed to be called when the devloper wanted
> > to do something specific with this type of dbm. so in the berkley DB
> > case there could be a apr_dbm_db_getRaw() function which returns the DB*
> > so that developers are not constrained the set of functions we chose to
> > abstract.
>
> No... we already have patterns for getting at underlying data types. See
> apr_portable.h. Follow that pattern *IF* we are to expose those. IMO, we
> should not do so.
>
> > (BTW I was planning on implmenting a shared-memory hash table, that I
> > posted ages ago, via this DBM interface, and it required a different set
> > of opening parameters (key/elem size, #elements) these parameters could
> > be passed via a db_set function call but it would look clunky
>
> No... keys and values are arbitrary length, as specified by an apr_datum_t.
> If your hash cannot handle that, then it cannot be used in the APR DBM
> interface.
>
> >...
> > so..
> > shall we put it to a vote?
> >
> > apr_dbm_open_XXX
> > or
> > apr_dbm_open(XXX)
>
> Registration and the second form.
>
>
> But your patch still has lots of work anyways. The GDBM functionality can't
> just disappear. Sorry, but the DAV functionality uses apr_dbm and you'd
> completely break access to the property files.
>
> Second, the vtable should not be part of the runtime apr_dbm_t structure.
> You should have a pointer to a static const table, like we do with buckets.
>
> The formatting may have gone thru "indent" but it still sucks. Just a glance
> over it shows some random gunk. For example, what the heck is up with the
> formatting of apr_datum_t in apr_dbm.h? And all the spacing of the function
> params in apr_dbm.h has been changed. There are extra blank lines at the end
> of some functions (after returns), and some missing blank lines between
> functions.
>
> We should also see a patch against apr_dbm.h, rather than the whole file. I
> can't see if you've changed the public interface or not.
>
> dbm_private.h is missing a license and disclaimer. It also has a
> non-standard #ifndef/#define/#endif pattern around it. We never start the
> symbol with "_", and on all the new .h files you specified: toss the "1"
> from the #define ... we don't do that either.
>
> Why is the "used names" functionality part of the per-db public interface?
> That should be part of the hook stuff just like the rest.
>
> In any case... the per-db headers should go, in favor of the registration
> mechanism. You would register each DB style with a name. You can then open a
> DB style by name, or using the reserved "any". Note that we would define
> some standard names:
>
> #define APR_DBM_USE_SDBM "sdbm"
> #define APR_DBM_USE_GDBM "gdbm"
> #define APR_DBM_USE_DB "db"
> #define APR_DBM_USE_ANY "any"
>
> Also, the whole point of this exercise was to link in all of the variations.
> That means the .c files should not have the #if APU_USE_*/#endif stuff
> around them. The APU_USE_* symbols are only used to determine which DB to
> use for the "any" case.
>
>
> So... -1 for now. The patch still needs work before it can be applied.
>
> Cheers,
> -g
>
Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time.
Posted by Greg Stein <gs...@lyra.org>.
-1 until you put that GDBM support back in. It isn't right to just drop it.
On Mon, Aug 20, 2001 at 04:50:25PM -0700, Ian Holsman wrote:
> On Mon, 2001-08-20 at 13:07, Sterling Hughes wrote:
>...
> > Not sure I understand what you're saying? There are ways to allow
> > the registering of module functionality, without modifying the main
> > package. Just provide a function like:
> >
> > apr_dbm_register()
> >
> > Which registers a dbm internally, and then have a constant defined
> > in a seperate file, and all is clear...
> that would work,
> but that wouldn't that mean there would be need to some init code
> somewhere to call the registration function?
Every time we load a module, we call registration functions. That is no big
deal.
Your solution of apr_dbm_*_open() still requires a recompilation/relink. The
registration mechanism is completely open-ended.
> > > 2. I may have a dbm which needs extra parameters to open it. (say for
> > > exampe cache size, or maybe it requires
> > > a fixed key length defined) I couldn't do this simply
> > >
> >
> > dbm_set_x() and check the flags, once they're all filled out (a
> > va_args implementation could also be done...)
Not sure what you mean by dbm_set_x() here. You have to open the thing first
to get an apr_dbm_t. But then you're too late.
The va_arg solution is a semi-reasonable solution, but that kills any chance
for type checking.
> > Furthermore, the idea of an abstraction layer is too bring all these
> > down to a lowest common denominator, at the cost of functionality
> > sometimes; usually functions that take extra arguments, can be
> > filled in with acceptable defaults. You'll run into the same
> > problems in that some db's don't support certain features, and
> > others do, the idea is to concentrate on the functionality that is
> > necessary and shared between the different db's (wrap this in a
> > large imho :).
Exactly. apr_dbm exposes DBM-style functionality. Simple key/value pairs.
That is it. The open functionality does not need flexibility. It has not
been demonstrated yet that we need more capability there.
> true, thats why they all return the same thing (apr_dbm_t)
> the open_DBM function was supposed to be called when the devloper wanted
> to do something specific with this type of dbm. so in the berkley DB
> case there could be a apr_dbm_db_getRaw() function which returns the DB*
> so that developers are not constrained the set of functions we chose to
> abstract.
No... we already have patterns for getting at underlying data types. See
apr_portable.h. Follow that pattern *IF* we are to expose those. IMO, we
should not do so.
> (BTW I was planning on implmenting a shared-memory hash table, that I
> posted ages ago, via this DBM interface, and it required a different set
> of opening parameters (key/elem size, #elements) these parameters could
> be passed via a db_set function call but it would look clunky
No... keys and values are arbitrary length, as specified by an apr_datum_t.
If your hash cannot handle that, then it cannot be used in the APR DBM
interface.
>...
> so..
> shall we put it to a vote?
>
> apr_dbm_open_XXX
> or
> apr_dbm_open(XXX)
Registration and the second form.
But your patch still has lots of work anyways. The GDBM functionality can't
just disappear. Sorry, but the DAV functionality uses apr_dbm and you'd
completely break access to the property files.
Second, the vtable should not be part of the runtime apr_dbm_t structure.
You should have a pointer to a static const table, like we do with buckets.
The formatting may have gone thru "indent" but it still sucks. Just a glance
over it shows some random gunk. For example, what the heck is up with the
formatting of apr_datum_t in apr_dbm.h? And all the spacing of the function
params in apr_dbm.h has been changed. There are extra blank lines at the end
of some functions (after returns), and some missing blank lines between
functions.
We should also see a patch against apr_dbm.h, rather than the whole file. I
can't see if you've changed the public interface or not.
dbm_private.h is missing a license and disclaimer. It also has a
non-standard #ifndef/#define/#endif pattern around it. We never start the
symbol with "_", and on all the new .h files you specified: toss the "1"
from the #define ... we don't do that either.
Why is the "used names" functionality part of the per-db public interface?
That should be part of the hook stuff just like the rest.
In any case... the per-db headers should go, in favor of the registration
mechanism. You would register each DB style with a name. You can then open a
DB style by name, or using the reserved "any". Note that we would define
some standard names:
#define APR_DBM_USE_SDBM "sdbm"
#define APR_DBM_USE_GDBM "gdbm"
#define APR_DBM_USE_DB "db"
#define APR_DBM_USE_ANY "any"
Also, the whole point of this exercise was to link in all of the variations.
That means the .c files should not have the #if APU_USE_*/#endif stuff
around them. The APU_USE_* symbols are only used to determine which DB to
use for the "any" case.
So... -1 for now. The patch still needs work before it can be applied.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/