You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by wr...@apache.org on 2001/05/09 21:12:45 UTC
cvs commit: apr-util/dbm/sdbm sdbm.c sdbm_lock.c sdbm_private.h
wrowe 01/05/09 12:12:44
Modified: include apr_sdbm.h
dbm/sdbm sdbm.c sdbm_lock.c sdbm_private.h
Log:
Introduce refcounted apr_sdbm_[un]lock() public functions and document
the whole interface.
Revision Changes Path
1.8 +132 -19 apr-util/include/apr_sdbm.h
Index: apr_sdbm.h
===================================================================
RCS file: /home/cvs/apr-util/include/apr_sdbm.h,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -r1.7 -r1.8
--- apr_sdbm.h 2001/05/06 12:58:05 1.7
+++ apr_sdbm.h 2001/05/09 19:12:33 1.8
@@ -62,14 +62,24 @@
#ifndef APR_SDBM_H
#define APR_SDBM_H
+#include "apu.h"
#include "apr_errno.h"
#include "apr_file_io.h" /* for apr_fileperms_t */
+/**
+ * @package apr-util SDBM library
+ */
+
+/**
+ * Structure for referencing an sdbm
+ * @defvar apr_sdbm_t
+ */
typedef struct apr_sdbm_t apr_sdbm_t;
-/* utility functions */
-int apr_sdbm_rdonly(apr_sdbm_t *db);
-
+/**
+ * Structure for referencing the datum record within an sdbm
+ * @defvar apr_sdbm_datum_t
+ */
typedef struct {
char *dptr;
int dsize;
@@ -78,25 +88,128 @@
/* The extensions used for the database files */
#define APR_SDBM_DIRFEXT ".dir"
#define APR_SDBM_PAGFEXT ".pag"
-
-/* Standard dbm interface */
-
-apr_status_t apr_sdbm_open(apr_sdbm_t **db, const char *filename,
- apr_int32_t flags, apr_fileperms_t perms,
- apr_pool_t *p);
-apr_status_t apr_sdbm_close(apr_sdbm_t *db);
-
-apr_status_t apr_sdbm_fetch(apr_sdbm_t *db, apr_sdbm_datum_t *val, apr_sdbm_datum_t key);
-apr_status_t apr_sdbm_delete(apr_sdbm_t *db, const apr_sdbm_datum_t key);
-
-/* * flags to sdbm_store */
+/* flags to sdbm_store */
#define APR_SDBM_INSERT 0
#define APR_SDBM_REPLACE 1
#define APR_SDBM_INSERTDUP 2
-apr_status_t apr_sdbm_store(apr_sdbm_t *db, apr_sdbm_datum_t key,
- apr_sdbm_datum_t value, int flags);
-apr_status_t apr_sdbm_firstkey(apr_sdbm_t *db, apr_sdbm_datum_t *key);
-apr_status_t apr_sdbm_nextkey(apr_sdbm_t *db, apr_sdbm_datum_t *key);
+
+/**
+ * Open an sdbm database by file name
+ * @param db The newly opened database
+ * @param name The sdbm file to open
+ * @param mode The flag values (APR_READ and APR_BINARY flags are implicit)
+ * <PRE>
+ * APR_WRITE open for read-write access
+ * APR_CREATE create the sdbm if it does not exist
+ * APR_TRUNCATE empty the contents of the sdbm
+ * APR_EXCL fail for APR_CREATE if the file exists
+ * APR_DELONCLOSE delete the sdbm when closed
+ * APR_SHARELOCK support locking across process/machines
+ * </PRE>
+ * @param perm Permissions to apply to if created
+ * @param pool The pool to use when creating the sdbm
+ * @deffunc apr_status_t apr_sdbm_open(apr_sdbm_t **db, const char *name, apr_int32_t mode, apr_fileperms_t perms, apr_pool_t *p)
+ * @tip The sdbm name is not a true file name, as sdbm appends suffixes
+ * for seperate data and index files.
+ */
+APU_DECLARE(apr_status_t) apr_sdbm_open(apr_sdbm_t **db, const char *name,
+ apr_int32_t mode,
+ apr_fileperms_t perms, apr_pool_t *p);
+
+/**
+ * Close an sdbm file previously opened by apr_sdbm_open
+ * @param db The database to close
+ * @deffunc apr_status_t apr_sdbm_close(apr_sdbm_t *db)
+ */
+APU_DECLARE(apr_status_t) apr_sdbm_close(apr_sdbm_t *db);
+
+/**
+ * Lock an sdbm database for concurency of multiple operations
+ * @param db The database to lock
+ * @param type The lock type
+ * <PRE>
+ * APR_FLOCK_SHARED
+ * APR_FLOCK_EXCLUSIVE
+ * </PRE>
+ * @deffunc apr_status_t apr_sdbm_lock(apr_sdbm_t *db, int type)
+ * @tip Calls to apr_sdbm_lock may be nested. All apr_sdbm functions
+ * perform implicit locking. Since an APR_FLOCK_SHARED lock cannot be
+ * portably promoted to an APR_FLOCK_EXCLUSIVE lock, apr_sdbm_store and
+ * apr_sdbm_delete calls will fail if an APR_FLOCK_SHARED lock is held.
+ * The apr_sdbm_lock call requires the database to be opened with the
+ * APR_SHARELOCK mode value.
+ */
+APU_DECLARE(apr_status_t) apr_sdbm_lock(apr_sdbm_t *db, int type);
+
+/**
+ * Release an sdbm lock previously aquired by apr_sdbm_lock
+ * @param db The database to unlock
+ * @deffunc apr_status_t apr_sdbm_unlock(apr_sdbm_t *db)
+ */
+APU_DECLARE(apr_status_t) apr_sdbm_unlock(apr_sdbm_t *db);
+
+/**
+ * Fetch an sdbm record value by key
+ * @param db The database
+ * @param value The value datum retrieved for this record
+ * @param key The key datum to find this record
+ * @deffunc apr_status_t apr_status_t apr_sdbm_fetch(apr_sdbm_t *db, apr_sdbm_datum_t *value, apr_sdbm_datum_t key)
+ */
+APU_DECLARE(apr_status_t) apr_sdbm_fetch(apr_sdbm_t *db,
+ apr_sdbm_datum_t *value,
+ apr_sdbm_datum_t key);
+
+/**
+ * Store an sdbm record value by key
+ * @param db The database
+ * @param key The key datum to store this record by
+ * @param value The value datum to store in this record
+ * @param opt The method used to store the record
+ * <PRE>
+ * APR_SDBM_INSERT return an error if the record exists
+ * APR_SDBM_REPLACE overwrite any existing record for key
+ * </PRE>
+ * @deffunc apr_status_t apr_sdbm_store(apr_sdbm_t *db, apr_sdbm_datum_t key, apr_sdbm_datum_t value, int opt)
+ */
+APU_DECLARE(apr_status_t) apr_sdbm_store(apr_sdbm_t *db, apr_sdbm_datum_t key,
+ apr_sdbm_datum_t value, int opt);
+
+/**
+ * Delete an sdbm record value by key
+ * @param db The database
+ * @param key The key datum of the record to delete
+ * @deffunc apr_status_t apr_sdbm_delete(apr_sdbm_t *db, const apr_sdbm_datum_t key)
+ * @tip It is not an error to delete a non-existent record.
+ */
+APU_DECLARE(apr_status_t) apr_sdbm_delete(apr_sdbm_t *db,
+ const apr_sdbm_datum_t key);
+
+/**
+ * Retrieve the first record key from a dbm
+ * @param dbm The database
+ * @param key The key datum of the first record
+ * @deffunc apr_status_t apr_sdbm_nextkey(apr_sdbm_t *db, apr_sdbm_datum_t *key)
+ * @tip The keys returned are not ordered. To traverse the list of keys
+ * for an sdbm opened with APR_SHARELOCK, the caller must use apr_sdbm_lock
+ * prior to retrieving the first record, and hold the lock until after the
+ * last call to apr_sdbm_nextkey.
+ */
+APU_DECLARE(apr_status_t) apr_sdbm_firstkey(apr_sdbm_t *db, apr_sdbm_datum_t *key);
+
+/**
+ * Retrieve the next record key from an sdbm
+ * @param db The database
+ * @param key The key datum of the next record
+ * @deffunc apr_status_t apr_sdbm_nextkey(apr_sdbm_t *db, apr_sdbm_datum_t *key)
+ */
+APU_DECLARE(apr_status_t) apr_sdbm_nextkey(apr_sdbm_t *db, apr_sdbm_datum_t *key);
+
+/**
+ * Returns true if the sdbm database opened for read-only access
+ * @param db The database to test
+ * @deffunc int apr_sdbm_rdonly(apr_sdbm_t *db)
+ */
+APU_DECLARE(int) apr_sdbm_rdonly(apr_sdbm_t *db);
#endif /* APR_SDBM_H */
1.21 +100 -46 apr-util/dbm/sdbm/sdbm.c
Index: sdbm.c
===================================================================
RCS file: /home/cvs/apr-util/dbm/sdbm/sdbm.c,v
retrieving revision 1.20
retrieving revision 1.21
diff -u -r1.20 -r1.21
--- sdbm.c 2001/05/06 13:21:17 1.20
+++ sdbm.c 2001/05/09 19:12:38 1.21
@@ -112,8 +112,13 @@
{
apr_sdbm_t *db = data;
+ /*
+ * Can't rely on apr_sdbm_unlock, since it will merely
+ * decrement the refcnt if several locks are held.
+ */
+ if (db->flags & (SDBM_SHARED_LOCK | SDBM_EXCLUSIVE_LOCK))
+ (void) apr_file_unlock(db->dirf);
(void) apr_file_close(db->dirf);
- (void) sdbm_unlock(db);
(void) apr_file_close(db->pagf);
free(db);
@@ -143,6 +148,17 @@
db->flags |= SDBM_RDONLY;
}
+ /*
+ * adjust the file open flags so that we handle locking
+ * on our own (don't rely on any locking behavior within
+ * an apr_file_t, in case it's ever introduced, and set
+ * our own flag.
+ */
+ if (flags & APR_SHARELOCK) {
+ db->flags |= SDBM_SHARED;
+ flags &= ~APR_SHARELOCK;
+ }
+
flags |= APR_BINARY | APR_READ;
/*
@@ -150,15 +166,17 @@
* If we fail anywhere, undo everything, return NULL.
*/
- if ((status = apr_file_open(&db->pagf, pagname, flags, perms, p))
+ if ((status = apr_file_open(&db->dirf, dirname, flags, perms, p))
!= APR_SUCCESS)
goto error;
- if ((status = sdbm_lock(db, !(db->flags & SDBM_RDONLY)))
+ if ((status = apr_file_open(&db->pagf, pagname, flags, perms, p))
!= APR_SUCCESS)
goto error;
- if ((status = apr_file_open(&db->dirf, dirname, flags, perms, p))
+ if ((status = apr_sdbm_lock(db, (db->flags & SDBM_RDONLY)
+ ? APR_FLOCK_SHARED
+ : APR_FLOCK_EXCLUSIVE))
!= APR_SUCCESS)
goto error;
@@ -173,12 +191,17 @@
* zero size: either a fresh database, or one with a single,
* unsplit data page: dirpage is all zeros.
*/
- db->dirbno = (!finfo.size) ? 0 : -1;
- db->pagbno = -1;
- db->maxbno = finfo.size * BYTESIZ;
+ SDBM_INVALIDATE_CACHE(db, finfo);
/* (apr_pcalloc zeroed the buffers) */
+ /*
+ * if we are opened in SHARED mode, unlock ourself
+ */
+ if (db->flags & SDBM_SHARED)
+ if ((status = apr_sdbm_unlock(db)) != APR_SUCCESS)
+ goto error;
+
/* make sure that we close the database at some point */
apr_pool_cleanup_register(p, db, database_cleanup, apr_pool_cleanup_null);
@@ -187,19 +210,20 @@
return APR_SUCCESS;
error:
+ if (db->dirf && db->pagf)
+ (void) apr_sdbm_unlock(db);
if (db->dirf != NULL)
(void) apr_file_close(db->dirf);
if (db->pagf != NULL) {
- (void) sdbm_unlock(db);
(void) apr_file_close(db->pagf);
}
free(db);
return status;
}
-apr_status_t apr_sdbm_open(apr_sdbm_t **db, const char *file,
- apr_int32_t flags, apr_fileperms_t perms,
- apr_pool_t *p)
+APU_DECLARE(apr_status_t) apr_sdbm_open(apr_sdbm_t **db, const char *file,
+ apr_int32_t flags,
+ apr_fileperms_t perms, apr_pool_t *p)
{
char *dirname = apr_pstrcat(p, file, APR_SDBM_DIRFEXT, NULL);
char *pagname = apr_pstrcat(p, file, APR_SDBM_PAGFEXT, NULL);
@@ -207,24 +231,29 @@
return prep(db, dirname, pagname, flags, perms, p);
}
-apr_status_t apr_sdbm_close(apr_sdbm_t *db)
+APU_DECLARE(apr_status_t) apr_sdbm_close(apr_sdbm_t *db)
{
return apr_pool_cleanup_run(db->pool, db, database_cleanup);
}
-apr_status_t apr_sdbm_fetch(apr_sdbm_t *db, apr_sdbm_datum_t *val,
- apr_sdbm_datum_t key)
+APU_DECLARE(apr_status_t) apr_sdbm_fetch(apr_sdbm_t *db, apr_sdbm_datum_t *val,
+ apr_sdbm_datum_t key)
{
apr_status_t status;
+
if (db == NULL || bad(key))
return APR_EINVAL;
+ if ((status = apr_sdbm_lock(db, APR_FLOCK_SHARED)) != APR_SUCCESS)
+ return status;
+
if ((status = getpage(db, exhash(key))) == APR_SUCCESS) {
*val = getpair(db->pagbuf, key);
/* ### do we want a not-found result? */
- return APR_SUCCESS;
}
+ (void) apr_sdbm_unlock(db);
+
return status;
}
@@ -239,42 +268,43 @@
return status;
}
-apr_status_t apr_sdbm_delete(apr_sdbm_t *db, const apr_sdbm_datum_t key)
+APU_DECLARE(apr_status_t) apr_sdbm_delete(apr_sdbm_t *db,
+ const apr_sdbm_datum_t key)
{
apr_status_t status;
-
+
if (db == NULL || bad(key))
return APR_EINVAL;
if (apr_sdbm_rdonly(db))
return APR_EINVAL;
+
+ if ((status = apr_sdbm_lock(db, APR_FLOCK_EXCLUSIVE)) != APR_SUCCESS)
+ return status;
if ((status = getpage(db, exhash(key))) == APR_SUCCESS) {
if (!delpair(db->pagbuf, key))
/* ### should we define some APRUTIL codes? */
- return APR_EGENERAL;
-
- /*
- * update the page file
- */
- status = write_page(db, db->pagbuf, db->pagbno);
- return status;
+ status = APR_EGENERAL;
+ else
+ status = write_page(db, db->pagbuf, db->pagbno);
}
+
+ (void) apr_sdbm_unlock(db);
- return APR_EACCES;
+ return status;
}
-apr_status_t apr_sdbm_store(apr_sdbm_t *db, apr_sdbm_datum_t key,
- apr_sdbm_datum_t val, int flags)
+APU_DECLARE(apr_status_t) apr_sdbm_store(apr_sdbm_t *db, apr_sdbm_datum_t key,
+ apr_sdbm_datum_t val, int flags)
{
int need;
register long hash;
apr_status_t status;
-
+
if (db == NULL || bad(key))
return APR_EINVAL;
if (apr_sdbm_rdonly(db))
return APR_EINVAL;
-
need = key.dsize + val.dsize;
/*
* is the pair too big (or too small) for this database ??
@@ -282,6 +312,9 @@
if (need < 0 || need > PAIRMAX)
return APR_EINVAL;
+ if ((status = apr_sdbm_lock(db, APR_FLOCK_EXCLUSIVE)) != APR_SUCCESS)
+ return status;
+
if ((status = getpage(db, (hash = exhash(key)))) == APR_SUCCESS) {
/*
@@ -290,21 +323,28 @@
*/
if (flags == APR_SDBM_REPLACE)
(void) delpair(db->pagbuf, key);
- else if (!(flags & APR_SDBM_INSERTDUP) && duppair(db->pagbuf, key))
- return APR_EEXIST;
+ else if (!(flags & APR_SDBM_INSERTDUP) && duppair(db->pagbuf, key)) {
+ status = APR_EEXIST;
+ goto error;
+ }
/*
* if we do not have enough room, we have to split.
*/
if (!fitpair(db->pagbuf, need))
if ((status = makroom(db, hash, need)) != APR_SUCCESS)
- return status;
+ goto error;
/*
* we have enough room or split is successful. insert the key,
* and update the page file.
*/
(void) putpair(db->pagbuf, key, val);
+
status = write_page(db, db->pagbuf, db->pagbno);
- }
+ }
+
+error:
+ (void) apr_sdbm_unlock(db);
+
return status;
}
@@ -405,6 +445,7 @@
status = APR_SUCCESS;
}
}
+
return status;
}
@@ -412,26 +453,37 @@
* the following two routines will break if
* deletions aren't taken into account. (ndbm bug)
*/
-apr_status_t apr_sdbm_firstkey(apr_sdbm_t *db, apr_sdbm_datum_t *key)
+APU_DECLARE(apr_status_t) apr_sdbm_firstkey(apr_sdbm_t *db,
+ apr_sdbm_datum_t *key)
{
+ apr_status_t status;
+
+ if ((status = apr_sdbm_lock(db, APR_FLOCK_SHARED)) != APR_SUCCESS)
+ return status;
+
/*
* start at page 0
*/
- apr_status_t status;
- if ((status = read_from(db->pagf, db->pagbuf, OFF_PAG(0), PBLKSIZ))
- != APR_SUCCESS)
- return status;
+ status = read_from(db->pagf, db->pagbuf, OFF_PAG(0), PBLKSIZ);
- db->pagbno = 0;
- db->blkptr = 0;
- db->keyptr = 0;
+ (void) apr_sdbm_unlock(db);
return getnext(key, db);
}
-apr_status_t apr_sdbm_nextkey(apr_sdbm_t *db, apr_sdbm_datum_t *key)
+APU_DECLARE(apr_status_t) apr_sdbm_nextkey(apr_sdbm_t *db,
+ apr_sdbm_datum_t *key)
{
- return getnext(key, db);
+ apr_status_t status;
+
+ if ((status = apr_sdbm_lock(db, APR_FLOCK_SHARED)) != APR_SUCCESS)
+ return status;
+
+ status = getnext(key, db);
+
+ (void) apr_sdbm_unlock(db);
+
+ return status;
}
/*
@@ -467,9 +519,8 @@
* ### we make it so in read_from anyway.
*/
if ((status = read_from(db->pagf, db->pagbuf, OFF_PAG(pagb), PBLKSIZ))
- != APR_SUCCESS) {
+ != APR_SUCCESS)
return status;
- }
if (!chkpage(db->pagbuf))
return APR_ENOSPC; /* ### better error? */
@@ -571,8 +622,11 @@
}
-int apr_sdbm_rdonly(apr_sdbm_t *db)
+APU_DECLARE(int) apr_sdbm_rdonly(apr_sdbm_t *db)
{
+ /* ### Should we return true if the first lock is a share lock,
+ * to reflect that apr_sdbm_store and apr_sdbm_delete will fail?
+ */
return (db->flags & SDBM_RDONLY) != 0;
}
1.7 +49 -9 apr-util/dbm/sdbm/sdbm_lock.c
Index: sdbm_lock.c
===================================================================
RCS file: /home/cvs/apr-util/dbm/sdbm/sdbm_lock.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- sdbm_lock.c 2001/04/30 17:16:19 1.6
+++ sdbm_lock.c 2001/05/09 19:12:40 1.7
@@ -52,25 +52,65 @@
* <http://www.apache.org/>.
*/
+#include "apr_file_info.h"
#include "apr_file_io.h"
#include "apr_sdbm.h"
#include "sdbm_private.h"
+#include "sdbm_tune.h"
/* NOTE: this function blocks until it acquires the lock */
-apr_status_t sdbm_lock(apr_sdbm_t *db, int exclusive)
+APU_DECLARE(apr_status_t) apr_sdbm_lock(apr_sdbm_t *db, int type)
{
- int type;
+ apr_status_t status;
- if (exclusive)
- type = APR_FLOCK_EXCLUSIVE;
- else
- type = APR_FLOCK_SHARED;
+ if (!(type == APR_FLOCK_SHARED || type == APR_FLOCK_EXCLUSIVE))
+ return APR_EINVAL;
- return apr_file_lock(db->pagf, type);
+ if (db->flags & SDBM_EXCLUSIVE_LOCK) {
+ ++db->lckcnt;
+ return APR_SUCCESS;
+ }
+ else if (db->flags & SDBM_SHARED_LOCK) {
+ /*
+ * Cannot promote a shared lock to an exlusive lock
+ * in a cross-platform compatibile manner.
+ */
+ if (type == APR_FLOCK_EXCLUSIVE)
+ return APR_EINVAL;
+ ++db->lckcnt;
+ return APR_SUCCESS;
+ }
+ /*
+ * zero size: either a fresh database, or one with a single,
+ * unsplit data page: dirpage is all zeros.
+ */
+ if ((status = apr_file_lock(db->dirf, type)) == APR_SUCCESS)
+ {
+ apr_finfo_t finfo;
+ if ((status = apr_file_info_get(&finfo, APR_FINFO_SIZE, db->dirf))
+ != APR_SUCCESS) {
+ (void) apr_file_unlock(db->dirf);
+ return status;
+ }
+
+ SDBM_INVALIDATE_CACHE(db, finfo);
+
+ ++db->lckcnt;
+ if (type == APR_FLOCK_SHARED)
+ db->flags |= SDBM_SHARED_LOCK;
+ else if (type == APR_FLOCK_EXCLUSIVE)
+ db->flags |= SDBM_EXCLUSIVE_LOCK;
+ }
+ return status;
}
-apr_status_t sdbm_unlock(apr_sdbm_t *db)
+APU_DECLARE(apr_status_t) apr_sdbm_unlock(apr_sdbm_t *db)
{
- return apr_file_unlock(db->pagf);
+ if (!(db->flags & (SDBM_SHARED_LOCK | SDBM_EXCLUSIVE_LOCK)))
+ return APR_EINVAL;
+ if (--db->lckcnt > 0)
+ return APR_SUCCESS;
+ db->flags &= ~(SDBM_SHARED_LOCK | SDBM_EXCLUSIVE_LOCK);
+ return apr_file_unlock(db->dirf);
}
1.7 +16 -6 apr-util/dbm/sdbm/sdbm_private.h
Index: sdbm_private.h
===================================================================
RCS file: /home/cvs/apr-util/dbm/sdbm/sdbm_private.h,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- sdbm_private.h 2001/05/06 12:58:05 1.6
+++ sdbm_private.h 2001/05/09 19:12:41 1.7
@@ -79,30 +79,40 @@
#define SPLTMAX 10 /* maximum allowed splits */
/* for apr_sdbm_t.flags */
-#define SDBM_RDONLY 0x1 /* data base open read-only */
+#define SDBM_RDONLY 0x1 /* data base open read-only */
+#define SDBM_SHARED 0x2 /* data base open for sharing */
+#define SDBM_SHARED_LOCK 0x4 /* data base locked for shared read */
+#define SDBM_EXCLUSIVE_LOCK 0x8 /* data base locked for write */
struct apr_sdbm_t {
apr_pool_t *pool;
apr_file_t *dirf; /* directory file descriptor */
apr_file_t *pagf; /* page file descriptor */
- apr_int32_t flags; /* status/error flags, see above */
+ apr_int32_t flags; /* status/error flags, see below */
long maxbno; /* size of dirfile in bits */
long curbit; /* current bit number */
long hmask; /* current hash mask */
long blkptr; /* current block for nextkey */
- int keyptr; /* current key for nextkey */
+ int keyptr; /* current key for nextkey */
long blkno; /* current page to read/write */
long pagbno; /* current page in pagbuf */
char pagbuf[PBLKSIZ]; /* page file block buffer */
long dirbno; /* current block in dirbuf */
char dirbuf[DBLKSIZ]; /* directory file block buffer */
+ int lckcnt; /* number of calls to sdbm_lock */
};
-apr_status_t sdbm_lock(apr_sdbm_t *db, int exclusive);
-apr_status_t sdbm_unlock(apr_sdbm_t *db);
-
extern const apr_sdbm_datum_t sdbm_nullitem;
long sdbm_hash(const char *str, int len);
+
+/*
+ * zero the cache
+ */
+#define SDBM_INVALIDATE_CACHE(db, finfo) \
+ do { db->dirbno = (!finfo.size) ? 0 : -1; \
+ db->pagbno = -1; \
+ db->maxbno = finfo.size * BYTESIZ; \
+ } while (0);
#endif /* SDBM_PRIVATE_H */
Re: cvs commit: apr-util/dbm/sdbm sdbm.c sdbm_lock.c sdbm_private.h
Posted by Greg Stein <gs...@lyra.org>.
I'll review this and the prior patch later tonite. Generally looks good, and
is certainly close enough that we can do minor tweaks (if needed) moving
forward.
Thanks for your patience!
Cheers,
-g
On Wed, May 09, 2001 at 07:12:45PM -0000, wrowe@apache.org wrote:
> wrowe 01/05/09 12:12:44
>
> Modified: include apr_sdbm.h
> dbm/sdbm sdbm.c sdbm_lock.c sdbm_private.h
> Log:
> Introduce refcounted apr_sdbm_[un]lock() public functions and document
> the whole interface.
>...
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apr-util/dbm/sdbm sdbm.c sdbm_lock.c sdbm_private.h
Posted by Greg Stein <gs...@lyra.org>.
On Wed, May 09, 2001 at 07:12:45PM -0000, wrowe@apache.org wrote:
> wrowe 01/05/09 12:12:44
>
> Modified: include apr_sdbm.h
> dbm/sdbm sdbm.c sdbm_lock.c sdbm_private.h
> Log:
> Introduce refcounted apr_sdbm_[un]lock() public functions and document
> the whole interface.
This patch and the previous looks good *except* for some very troubling
changes in apr_sdbm_firstkey()...
>...
> --- sdbm.c 2001/05/06 13:21:17 1.20
> +++ sdbm.c 2001/05/09 19:12:38 1.21
>...
> @@ -412,26 +453,37 @@
> * the following two routines will break if
> * deletions aren't taken into account. (ndbm bug)
> */
> -apr_status_t apr_sdbm_firstkey(apr_sdbm_t *db, apr_sdbm_datum_t *key)
> +APU_DECLARE(apr_status_t) apr_sdbm_firstkey(apr_sdbm_t *db,
> + apr_sdbm_datum_t *key)
> {
> + apr_status_t status;
> +
> + if ((status = apr_sdbm_lock(db, APR_FLOCK_SHARED)) != APR_SUCCESS)
> + return status;
> +
> /*
> * start at page 0
> */
> - apr_status_t status;
> - if ((status = read_from(db->pagf, db->pagbuf, OFF_PAG(0), PBLKSIZ))
> - != APR_SUCCESS)
> - return status;
> + status = read_from(db->pagf, db->pagbuf, OFF_PAG(0), PBLKSIZ);
>
> - db->pagbno = 0;
> - db->blkptr = 0;
> - db->keyptr = 0;
> + (void) apr_sdbm_unlock(db);
>
> return getnext(key, db);
> }
Why did you delete the db->pagbno line and the following two? That looks
very wrong. Those aren't the same bits as what the SDBM_INVALIDATE_CACHE
does (I'm thinking maybe you removed them thinking the invalidate would do
the same thing?)
Also, shouldn't the sequence be:
status = getnext(key, db);
(void) apr_sdbm_unlock(db);
?? You use that sequence in apr_sdbm_nextkey(). It also seems a bit safer to
do the getnext() within the lock.
>...
> +APU_DECLARE(apr_status_t) apr_sdbm_lock(apr_sdbm_t *db, int type)
>...
> + /*
> + * zero size: either a fresh database, or one with a single,
> + * unsplit data page: dirpage is all zeros.
> + */
Eh? What is that comment doing there?!
>...
> + SDBM_INVALIDATE_CACHE(db, finfo);
Okay... I finally figured out how/why this can make sense. Basically, the
semantic is, "I just got a lock; any data that I may have had is bogus." And
since we make sure to have a lock whenever we do something, this will ensure
that stuff is tossed during the NOLOCK -> HAVELOCK transition.
Works for me :-)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/