On Wed, May 09, 2001 at 07:12:45PM -0000, [EMAIL PROTECTED] 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/
