libmemcache, various minor questions

[John McCaskey]

Ok, again really sorry for the flood... 

but in addition in mc_res_free(), doesn't the following result in a free
if either MC_RES_FREE_ON_DELETE or MC_RES_NO_FREE_ON_DELETE are set?
making the flag essentially always set to delete?  (lines starting at
1041)

if (res->_flags & (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);
  }

After I modified this, and the previous item mentioned below, mc_aget
appears to work fine.  However I'm not sure if this has negative side
effects like creating a memory leak on accident because maybe something
else relied on this behavior ;)

Again, thanks for all the work Sean.  I personally think the mc_get
interface is cleaner and will be using it anyway.  But, if what I've
stated here seems to make sense I'll be happy to put it all into a patch
and submit to you tommorow.

On Mon, 2004-12-06 at 17:35 -0800, John McCaskey wrote:
> Ok, sorry for the flood of emails :)
> 
> 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.
> 
> 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.  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?
> 
> On Mon, 2004-12-06 at 16:46 -0800, John McCaskey wrote:
> > Here is code to reproduce my mc_aget issue:
> > 
> > /* This is a test program for the memcache C API */
> > 
> > #include <err.h>
> > #include <sysexits.h>
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > #include <unistd.h>
> > 
> > #include "libmemcache.h"
> > 
> > void test_add(struct memcache *mc, const u_int32_t);
> > void test_aget(struct memcache *mc, const char *key, const u_int32_t);
> > 
> > int
> > main(int argc, char *argv[]) {
> >   struct memcache *mc = NULL;
> >   mc = mc_new();
> >   if (mc == NULL)
> >     err(EX_OSERR, "Unable to allocate a new memcache object");
> > 
> >   mc_server_add(mc, "127.0.0.1", "21211");
> > 
> >   mc_add(mc, "foo", strlen("foo"), "test", strlen("test"), 0, 0);
> > 
> >   test_aget(mc, "foo", 5);
> > 
> >   mc_free(mc);
> > 
> >   return EX_OK;
> > }
> > 
> > 
> > void
> > test_aget(struct memcache *mc, const char *key, const u_int32_t count) {
> >   void *val;
> >   u_int32_t i;
> > 
> >   for (i = 0; i < count; i++) {
> >     val = mc_aget(mc, key, strlen(key));
> >     if (val != NULL) {
> >       printf("%s", (char *)val);
> >       free(val);
> >     }
> >   }
> > }
> > 
> > 
> > Using this code foo get stored correctly as "test", and can later be
> > retrieved by other methods, but the mc_aget method returns seemingly
> > random memory addresses that do not match up.
> > 
> > Am I misunderstanding how I should use mc_aget?
> > 
> > 
> > On Mon, 2004-12-06 at 16:28 -0800, John McCaskey wrote:
> > > With gcc under linux I get alot of:
> > > 
> > > external/libmemcache/libmemcache.c:208: warning: assignment discards
> > > qualifiers from pointer target type
> > > 
> > > type warnings, looks like all it takes to avoid these is to explicitly
> > > cast away the const qualifier.  Is there a reason why this isn't being
> > > done (doesn't work on other platforms maybe?)?  I know the warning is
> > > nothing to worry about, but I just hate seeing them in my otherwise
> > > warning free code :)
> > > 
> > > Also, it looks like the examples on the libmemcache page need some
> > > updating, the order of arguments to mc_aget for example is incorrect!
> > > 
> > > And finally, I seemed to have alot of trouble using mc_aget with the
> > > correct value not being returned, and ultimately freeing the memory
> > > resulting in stack corruption.  I need to isolate out my code and try to
> > > reproduce this with a simple test case, if I can I'll report back.  When
> > > I swithced to using mc_req_add, mc_get, etc, I had no further issues.
> > > 
> > > Any known issues with mc_aget I should be aware of?
> > > 
-- 
John A. McCaskey
Software Development Engineer
Klir Technologies, Inc.
johnm at klir.com
206.902.2027