apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@covalent.net>
Subject Re: [proposal] DBM - allow multiple DBM's of differnt types at thesame time. -- V2
Date Tue, 20 Nov 2001 16:08:23 GMT
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" <ianh@cnet.com>
To: "Greg Stein" <gstein@lyra.org>
Cc: <dev@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
> 


Mime
View raw message