<<< Date Index >>>     <<< Thread Index >>>

Re: hcache broken (slightly)



Hi,

* Thomas Roessler [06-07-11 16:27:00 -0400] wrote:
On 2006-07-11 20:19:56 +0000, Rocco Rutte wrote:

One reason could be the way data is stored: it's just a
memcpy of the HEADER* structure with pointers and such.
However, when we first fetch a header, it doesn't have the
full information like pointers set up for sorting etc. Maybe
that could be the reason?

Sounds like a plausible one, no?

Yeah, though in the first place this was a very wild guess. Now I'm even more confused that simple initialization causes everything to work like a charm in my first test.

If the attached patch works, we can start using hcache for syncing right after 1.5.12 release. I now can even restore full flags for POP from hcache (including replied and all, nice) (except setting flags doesn't work yet due to pending ACL patch, I can change, sync and restore the new flag).

I ask for other ideas before I try to zero them out in
hcache.c after fetching a header.

That would be a reasonable thing to do, I think...  Copying
pointers from previous instances of mutt seems dangerous.

ACK. It even left driver-specific data pointers ('void*') in. Brendan suggested to zero the pointers in hcache_put(); but looking at the HEADER definition, there much more things computed at runtime according to local settings. So now my approach is to only explicitely copy all flags/values we certainly want ignoring all others (and thus triggering re-creation as needed).

Though it may not be too obvious afterall, I think this approach has another great advantage. We need to force commits on hcache.c with every structure change anyway (due to CVS '$Id$' being used for CRC check), so having the members of HEADER explicitely in hcache.c could help.

  bye, Rocco
--
:wq!
diff --git a/hcache.c b/hcache.c
index d4b8f2a..6fb15ab 100644
--- a/hcache.c
+++ b/hcache.c
@@ -570,6 +570,8 @@ mutt_hcache_dump(header_cache_t *h, HEAD
                 unsigned long uid_validity)
 {
   unsigned char *d = NULL;
+  HEADER tmp;
+
   *off = 0;
 
   d = lazy_malloc(sizeof (validate));
@@ -587,7 +589,26 @@ mutt_hcache_dump(header_cache_t *h, HEAD
   d = dump_int(h->crc, d, off);
 
   lazy_realloc(&d, *off + sizeof (HEADER));
-  memcpy(d + *off, header, sizeof (HEADER));
+
+  /* copy the interesting bits we want, leave others 0 */
+  memset (&tmp, 0, sizeof (HEADER));
+  tmp.security = header->security; 
+  tmp.mime = header->mime; 
+  tmp.flagged = header->flagged; 
+  tmp.old = header->old; 
+  tmp.read = header->read; 
+  tmp.expired = header->expired; 
+  tmp.superseded = header->superseded; 
+  tmp.replied = header->replied; 
+  tmp.recip_valid = header->recip_valid; 
+  tmp.zhours = header->zhours; 
+  tmp.zminutes = header->zminutes; 
+  tmp.zoccident = header->zoccident; 
+  tmp.date_sent = header->date_sent; 
+  tmp.received = header->received; 
+  tmp.lines = header->lines; 
+
+  memcpy(d + *off, &tmp, sizeof (HEADER));
   *off += sizeof (HEADER);
 
   d = dump_envelope(header->env, d, off);
diff --git a/pop.c b/pop.c
index 72925c7..1620568 100644
--- a/pop.c
+++ b/pop.c
@@ -195,7 +195,7 @@ static int msg_cache_check (const char *
 static int pop_fetch_headers (CONTEXT *ctx)
 {
   int i, ret, old_count, new_count;
-  unsigned short hcached = 0, bcached;
+  unsigned short hcached = 0;
   POP_DATA *pop_data = (POP_DATA *)ctx->data;
 
 #ifdef USE_HCACHE
@@ -285,30 +285,16 @@ #endif
 
       /*
        * faked support for flags works like this:
-       * - if 'hcached' is 1, we have the message in our hcache:
-       *        - if we also have a body: read
-       *        - if we don't have a body: old
-       *          (if $mark_old is set which is maybe wrong as
-       *          $mark_old should be considered for syncing the
-       *          folder and not when opening it XXX)
+       * - if 'hcached' is 1, take flags from hcache
        * - if 'hcached' is 0, we don't have the message in our hcache:
        *        - if we also have a body: read
        *        - if we don't have a body: new
        */
-      bcached = mutt_bcache_exists (pop_data->bcache, ctx->hdrs[i]->data) == 0;
-      ctx->hdrs[i]->old = 0;
-      ctx->hdrs[i]->read = 0;
-      if (hcached)
-      {
-        if (bcached)
-          ctx->hdrs[i]->read = 1;
-        else if (option (OPTMARKOLD))
-          ctx->hdrs[i]->old = 1;
-      }
-      else
+      if (!hcached)
       {
-        if (bcached)
-          ctx->hdrs[i]->read = 1;
+        ctx->hdrs[i]->old = 0;
+        ctx->hdrs[i]->read = mutt_bcache_exists (pop_data->bcache,
+                                                ctx->hdrs[i]->data) == 0;
       }
 
       ctx->msgcount++;
@@ -625,6 +611,10 @@ #if USE_HCACHE
 #endif
        }
       }
+#if USE_HCACHE
+      if (ctx->hdrs[i]->changed)
+       mutt_hcache_store (hc, ctx->hdrs[i]->data, ctx->hdrs[i], 0, strlen);
+#endif
     }
 
 #if USE_HCACHE