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 <ia...@cnet.com> on 2001/08/20 18:31:51 UTC
[proposal] DBM - allow multiple DBM's of differnt types at the same time.
I'm posting the source, as the patches would be too hard to read
I've split up the single apr_dbm.c into 2 parts.
a logical interface, (apr_dbm.c)
and a implementation. (apr_dbm_[SDBM/DB].c)
by doing this It allows:
* other DB types to be added easily externally by other people
* allows a 'default' db type for developers who just want a 'db'
* allows developers to specify which DB they want
* allows people to have multiple DB-types running at the same type.
If I could get some +1 votes I'll commit the changes
(and get a GDBM version working)
..Ian
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/
Re: [proposal] DBM - allow multiple DBM's of differnt types at the
same time.
Posted by Ian Holsman <in...@webperf.org>.
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.
Posted by Sterling Hughes <st...@designmultimedia.com>.
> >
> >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...
> 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 :).
-Sterling
Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time.
Posted by Ian Holsman <ia...@cnet.com>.
Ryan Bloom wrote:
>On Monday 20 August 2001 11:46, Cliff Woolley wrote:
>
>>On Mon, 20 Aug 2001, Sterling Hughes wrote:
>>
>>>>yes.. thats the point of the patch
>>>>#include <apr_dbm_sdbm.h>
>>>>#include <apr_dbm_db.h>
>>>>
>>>>apr_dbm_t b1= apr_dbm_open_sdbm(...);
>>>>apr_dbm_t b2= apr_dbm_open_db(...);
>>>>
>>>>should work.
>>>>the 'test' program will have a update later which will allow it to
>>>>choose DB types (-t DB|SDBM|GDBM)
>>>>
>>> Ahh, ok... I was thinking something more inline with:
>>>
>>> #include <apr_dbm.h>
>>>
>>> apr_dmb_t b1 = apr_dbm_open(APR_GDBM, ...);
>>>
>>> But either way works.
>>>
>>Different parts of APR and apr-util use one method or the other, so it's
>>hard to say which is more consistent. Some use Ian's method (eg buckets,
>>sms), some use yours (eg locks). I think I like Ian's flavor (the
>>apr_dbm_open_foo one) better in this context because it's probably more
>>easily extensible by an outside module that might know about some flavor
>>of dbm we don't know about. But that's just me. <shrug>
>>
>
>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
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
>
>Ryan
>
>______________________________________________________________
>Ryan Bloom rbb@apache.org
>Covalent Technologies rbb@covalent.net
>--------------------------------------------------------------
>
Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time.
Posted by Ryan Bloom <rb...@covalent.net>.
On Monday 20 August 2001 11:46, Cliff Woolley wrote:
> On Mon, 20 Aug 2001, Sterling Hughes wrote:
> > > yes.. thats the point of the patch
> > > #include <apr_dbm_sdbm.h>
> > > #include <apr_dbm_db.h>
> > >
> > > apr_dbm_t b1= apr_dbm_open_sdbm(...);
> > > apr_dbm_t b2= apr_dbm_open_db(...);
> > >
> > > should work.
> > > the 'test' program will have a update later which will allow it to
> > > choose DB types (-t DB|SDBM|GDBM)
> >
> > Ahh, ok... I was thinking something more inline with:
> >
> > #include <apr_dbm.h>
> >
> > apr_dmb_t b1 = apr_dbm_open(APR_GDBM, ...);
> >
> > But either way works.
>
> Different parts of APR and apr-util use one method or the other, so it's
> hard to say which is more consistent. Some use Ian's method (eg buckets,
> sms), some use yours (eg locks). I think I like Ian's flavor (the
> apr_dbm_open_foo one) better in this context because it's probably more
> easily extensible by an outside module that might know about some flavor
> of dbm we don't know about. But that's just me. <shrug>
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.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: [proposal] DBM - allow multiple DBM's of differnt types at the
same time.
Posted by Cliff Woolley <cl...@yahoo.com>.
On Mon, 20 Aug 2001, Sterling Hughes wrote:
> > yes.. thats the point of the patch
> > #include <apr_dbm_sdbm.h>
> > #include <apr_dbm_db.h>
> >
> > apr_dbm_t b1= apr_dbm_open_sdbm(...);
> > apr_dbm_t b2= apr_dbm_open_db(...);
> >
> > should work.
> > the 'test' program will have a update later which will allow it to
> > choose DB types (-t DB|SDBM|GDBM)
> >
>
> Ahh, ok... I was thinking something more inline with:
>
> #include <apr_dbm.h>
>
> apr_dmb_t b1 = apr_dbm_open(APR_GDBM, ...);
>
> But either way works.
Different parts of APR and apr-util use one method or the other, so it's
hard to say which is more consistent. Some use Ian's method (eg buckets,
sms), some use yours (eg locks). I think I like Ian's flavor (the
apr_dbm_open_foo one) better in this context because it's probably more
easily extensible by an outside module that might know about some flavor
of dbm we don't know about. But that's just me. <shrug>
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: [proposal] DBM - allow multiple DBM's of differnt types at the
same time.
Posted by Sterling Hughes <st...@designmultimedia.com>.
On Mon, 20 Aug 2001, Ian Holsman wrote:
> Sterling Hughes wrote:
>
> >
> >
> >>sorry bout that.
> >>MSVC does it's own thing with spacing ;(
> >>(ok.. I've run the code through indent so that should help.)
> >>
> >
> > It looks good -- a couple of comments:
> >
> > 1) Perhaps sync and optimize API functions could be added, yes,
> > some db's don't support this, but they can just do nothing for those
> > db's (no loss if there is no API for it).
> >
> sounds fair..
>
> >
> >
> > 2) Sometimes people may want to compile with support for two
> > databases and then decide at runtime which one they want to use.
> > Could this not be put into the current implementation?
> >
> yes.. thats the point of the patch
> #include <apr_dbm_sdbm.h>
> #include <apr_dbm_db.h>
>
> apr_dbm_t b1= apr_dbm_open_sdbm(...);
> apr_dbm_t b2= apr_dbm_open_db(...);
>
> should work.
> the 'test' program will have a update later which will allow it to
> choose DB types (-t DB|SDBM|GDBM)
>
Ahh, ok... I was thinking something more inline with:
#include <apr_dbm.h>
apr_dmb_t b1 = apr_dbm_open(APR_GDBM, ...);
But either way works.
> >
> > 3) Some of the Berkeley DB stuff doesn't look quite right to me,
> > however, I'm no expert (could be perfectly fine). Someone else should
> > probably take a closer look at this though....
> >
> I ran the test program on berkley 3.3.11 on NT, and the basic ops work
> (I don't think the current stuff does)
>
Ok, I took a look at the current implementation -- which seems even
iffier :)
-Sterling
Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time.
Posted by Ian Holsman <ia...@cnet.com>.
Sterling Hughes wrote:
>
>
>>sorry bout that.
>>MSVC does it's own thing with spacing ;(
>>(ok.. I've run the code through indent so that should help.)
>>
>
> It looks good -- a couple of comments:
>
> 1) Perhaps sync and optimize API functions could be added, yes,
> some db's don't support this, but they can just do nothing for those
> db's (no loss if there is no API for it).
>
sounds fair..
>
>
> 2) Sometimes people may want to compile with support for two
> databases and then decide at runtime which one they want to use.
> Could this not be put into the current implementation?
>
yes.. thats the point of the patch
#include <apr_dbm_sdbm.h>
#include <apr_dbm_db.h>
apr_dbm_t b1= apr_dbm_open_sdbm(...);
apr_dbm_t b2= apr_dbm_open_db(...);
should work.
the 'test' program will have a update later which will allow it to
choose DB types (-t DB|SDBM|GDBM)
>
> 3) Some of the Berkeley DB stuff doesn't look quite right to me,
> however, I'm no expert (could be perfectly fine). Someone else should
> probably take a closer look at this though....
>
I ran the test program on berkley 3.3.11 on NT, and the basic ops work
(I don't think the current stuff does)
>
> -Sterling
>
Re: [proposal] DBM - allow multiple DBM's of differnt types at the
same time.
Posted by Sterling Hughes <st...@designmultimedia.com>.
> sorry bout that.
> MSVC does it's own thing with spacing ;(
> (ok.. I've run the code through indent so that should help.)
>
It looks good -- a couple of comments:
1) Perhaps sync and optimize API functions could be added, yes,
some db's don't support this, but they can just do nothing for those
db's (no loss if there is no API for it).
2) Sometimes people may want to compile with support for two
databases and then decide at runtime which one they want to use.
Could this not be put into the current implementation?
3) Some of the Berkeley DB stuff doesn't look quite right to me,
however, I'm no expert (could be perfectly fine). Someone else should
probably take a closer look at this though....
-Sterling
Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time.
Posted by Ian Holsman <ia...@cnet.com>.
sorry bout that.
MSVC does it's own thing with spacing ;(
(ok.. I've run the code through indent so that should help.)
Justin Erenkrantz wrote:
>On Mon, Aug 20, 2001 at 09:31:51AM -0700, Ian Holsman wrote:
>
>>APU_DECLARE(apr_status_t) apr_dbm_fetch(apr_dbm_t *dbm, apr_datum_t key,
>> apr_datum_t *pvalue)
>>{
>> return dbm->fetch (dbm,key,pvalue);
>>}
>>
>
>Watch the style. This should be:
>
>return dbm->fetch(dbm, key, pvalue);
>
>No spaces before the ( and spaces in between parameter names.
>
>I think you misnamed the Berkeley DB file/structure. You called it
>berkley. =-) I can never spell it right either - I had to go to
>their website (which of course requires spelling berkeley right - a
>catch-22 - thanks goodness for Google...).
>
>>static void apr_dbm_close_db(apr_dbm_t *dbm)
>>{
>> //APR_DBM_CLOSE(dbm.file);
>>
>
>// is invalid in C. Use /* */.
>
>>static apr_status_t apr_dbm_fetch_db(apr_dbm_t *dbm, apr_datum_t key,
>> apr_datum_t *pvalue)
>>{
>> apr_status_t rv;
>> apr_berkley_db_t *real_ptr = dbm->file;
>> DB*db=real_ptr->bdb;
>> DBT ckey;
>> DBT rd;
>> int dberr;
>>
>
>Seems like there are tabs and spaces in this fragment. We should only
>have 4 space indention (no tab chars).
>
>HTH. Please repost once the style is cleaned-up and then I'll look at
>it again. My brain doesn't parse code that isn't in the correct
>style. Sorry.
>
>It generally looks good, but I'll take a deeper look once the style
>issues are addressed. -- justin
>
Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time.
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Mon, Aug 20, 2001 at 09:31:51AM -0700, Ian Holsman wrote:
> APU_DECLARE(apr_status_t) apr_dbm_fetch(apr_dbm_t *dbm, apr_datum_t key,
> apr_datum_t *pvalue)
> {
> return dbm->fetch (dbm,key,pvalue);
> }
Watch the style. This should be:
return dbm->fetch(dbm, key, pvalue);
No spaces before the ( and spaces in between parameter names.
I think you misnamed the Berkeley DB file/structure. You called it
berkley. =-) I can never spell it right either - I had to go to
their website (which of course requires spelling berkeley right - a
catch-22 - thanks goodness for Google...).
> static void apr_dbm_close_db(apr_dbm_t *dbm)
> {
> //APR_DBM_CLOSE(dbm.file);
// is invalid in C. Use /* */.
> static apr_status_t apr_dbm_fetch_db(apr_dbm_t *dbm, apr_datum_t key,
> apr_datum_t *pvalue)
> {
> apr_status_t rv;
> apr_berkley_db_t *real_ptr = dbm->file;
> DB*db=real_ptr->bdb;
> DBT ckey;
> DBT rd;
> int dberr;
Seems like there are tabs and spaces in this fragment. We should only
have 4 space indention (no tab chars).
HTH. Please repost once the style is cleaned-up and then I'll look at
it again. My brain doesn't parse code that isn't in the correct
style. Sorry.
It generally looks good, but I'll take a deeper look once the style
issues are addressed. -- justin