apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ian Holsman <i...@webperf.org>
Subject Re: [proposal] DBM - allow multiple DBM's of differnt types at the same time.
Date Wed, 22 Aug 2001 15:00:17 GMT
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
> 


Mime
View raw message