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 >
