libmemcache, various minor questions
Sean Chittenden
sean at chittenden.org
Tue Dec 7 11:25:21 PST 2004
> I believe that the following lines in libmemcache.c are causing free on
> delete to always be set when using mc_aget()...
>
> (lines 482, 483)
> if (res->_flags & (MC_RES_FREE_ON_DELETE | MC_RES_NO_FREE_ON_DELETE))
> mc_res_free_on_delete(res, (res->size > 0 ? 0 : 1));
>
>
> Looks like if either free_on_delete, or no_free_on delete was set, then
> the check based off res->size occurs, res->size seems to always be 0 at
> this point when using mc_aget, so free_on_delete gets reset to be on,
> even though it was explicitly set off just moments ago in the mc_aget
> function.
I'm using (MC_RES_FREE_ON_DELETE | MC_RES_NO_FREE_ON_DELETE) as the
default value meaning it hasn't been set and the library should try to
automatically detect if it should free on delete or not. Imagine the
following:
> struct memcache_res *res = mc_req_add(req, key3, key3_len);
> res->size = 1024; /* Allocate our own memory a head of time (useful
> for loops) */
> res->val = malloc(res->size);
> mc_get(mc, req);
> mc_res_free(req, res);
The 'res->size = 1024' bit tells libmemcache(3) to not worry about the
allocation/deallocation of res->val. One can also call
> mc_res_free_on_delete(res, 1);
so that when someone does
> mc_res_free(req, res);
it will free res and res->val.
> Am I right? I'm not clear to me what should be done about this,
> because
> I think the check is there to allow the memory pre-allocation that I
> see
> in some of your examples, but on the other hand it looks like it's
> causing all users of mc_aget to recieve a pointer to already freed
> memory.
The problem that I ran into (and I pushed this out without much
testing, I apologize), was that I needed a way of detecting successful
fetches of keys that had zero length responses. Originally I was
testing for valid keys based off of res->size, but this doesn't work in
the previous situation. So instead I changed the previous
res->free_on_delete to res->_flags that was a bit options field. As a
result, one can now fetch keys with zero length... but apparently I
didn't do this 100% right. I don't use mc_aget() anywhere any more in
my apps and didn't run my little regression test (stupid, yeah, I
know... normally I do but it was late and I wanted to leave the colo).
> Perhaps rather than trying to autodetect this users who are
> preallocating memory should simply have to call mc_res_free_on_delete
> themselves, and then the value of the flags should never get modified
> by
> this apparent auto-detection routine?
Well, that's just it. mc_res_free_on_delete(3) sets the value to
either one bit or the other, not both. If both bits are set, however,
then it means that the library needs to look at res->size to see if it
should free on delete or not. The logic is simple, but the coding
wasn't 100% correct. If you apply the following:
Index: memcache.c
===================================================================
RCS file: /opt/cvsroot/nexadesic/src/lib/libmemcache/memcache.c,v
retrieving revision 1.1
diff -u -r1.1 memcache.c
--- memcache.c 7 Dec 2004 01:17:11 -0000 1.1
+++ memcache.c 7 Dec 2004 19:08:20 -0000
@@ -493,7 +493,8 @@
/* While we're looping, might as well see if we should be auto
* deleting any of these keys. */
- if (res->_flags & (MC_RES_FREE_ON_DELETE |
MC_RES_NO_FREE_ON_DELETE))
+ if (res->_flags & (MC_RES_FREE_ON_DELETE |
MC_RES_NO_FREE_ON_DELETE) ==
+ (MC_RES_FREE_ON_DELETE | MC_RES_FREE_ON_DELETE))
mc_res_free_on_delete(res, (res->size > 0 ? 0 : 1));
}
rv[i].iov_base = str_endl;
@@ -686,7 +687,8 @@
/* Make sure we are where we think we are. */
if (mc->start[rb - 2] != '\r' && mc->start[rb - 1] != '\n') {
warnx("%s:%u\tProtocol Error", __FILE__, __LINE__);
- if (!(res->_flags & (MC_RES_FREE_ON_DELETE |
MC_RES_NO_FREE_ON_DELETE)) ||
+ if ((res->_flags & (MC_RES_FREE_ON_DELETE |
MC_RES_NO_FREE_ON_DELETE) ==
+ (MC_RES_FREE_ON_DELETE | MC_RES_NO_FREE_ON_DELETE)) ||
(res->_flags & MC_RES_FREE_ON_DELETE)) {
mcFree(res->val);
res->val = NULL;
@@ -1072,7 +1074,8 @@
void
mc_res_free(struct memcache_req *req, struct memcache_res *res) {
TAILQ_REMOVE(&req->query, res, entries);
- if (res->_flags & (MC_RES_FREE_ON_DELETE | MC_RES_NO_FREE_ON_DELETE)
||
+ if ((res->_flags & (MC_RES_FREE_ON_DELETE |
MC_RES_NO_FREE_ON_DELETE) ==
+ (MC_RES_FREE_ON_DELETE | MC_RES_NO_FREE_ON_DELETE)) ||
res->_flags & MC_RES_FREE_ON_DELETE) {
if (res->size > 0)
mcFree(res->val);
you should be good to go. Or, conversely, you could get the 1.0.1
release which can be found here:
http://people.FreeBSD.org/~seanc/libmemcache/
-sc
--
Sean Chittenden
More information about the memcached
mailing list