* Re: [9front] upas/fs imap fixes and improvements.
@ 2019-11-20 7:34 cinap_lenrek
0 siblings, 0 replies; 5+ messages in thread
From: cinap_lenrek @ 2019-11-20 7:34 UTC (permalink / raw)
To: 9front
on first glance:
- if(imap->nuid < imap->muid)
+ if(idx < imap->muid)
imap->f[imap->nuid].dates = l;
should be: imap->f[idx].dates = l instead?
+ memcpy(&imap->f[idx], &imap->f[idx + 1], imap->nmsg - idx - 1);
maybe use memmove() instead as source and dest overlap.
--
cinap
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [9front] upas/fs imap fixes and improvements.
@ 2019-11-22 23:46 ori
0 siblings, 0 replies; 5+ messages in thread
From: ori @ 2019-11-22 23:46 UTC (permalink / raw)
To: cinap_lenrek, 9front
> ok, no further nitpicks. lgtm, good luck! :)
>
> --
> cinap
Cool -- I'm reasonably confident in this, been using it
for the last week. But I won't be around to fix it if
that confidence is misplaced, so I'll land it on Dec 2nd.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [9front] upas/fs imap fixes and improvements.
@ 2019-11-22 20:50 cinap_lenrek
0 siblings, 0 replies; 5+ messages in thread
From: cinap_lenrek @ 2019-11-22 20:50 UTC (permalink / raw)
To: 9front
ok, no further nitpicks. lgtm, good luck! :)
--
cinap
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [9front] upas/fs imap fixes and improvements.
@ 2019-11-22 19:49 ori
0 siblings, 0 replies; 5+ messages in thread
From: ori @ 2019-11-22 19:49 UTC (permalink / raw)
To: ori, cinap_lenrek, 9front
>> on first glance:
>>
>> - if(imap->nuid < imap->muid)
>> + if(idx < imap->muid)
>> imap->f[imap->nuid].dates = l;
>>
>> should be: imap->f[idx].dates = l instead?
>>
>> + memcpy(&imap->f[idx], &imap->f[idx + 1], imap->nmsg - idx - 1);
>>
>> maybe use memmove() instead as source and dest overlap.
>>
>> --
>> cinap
>
> Fixed. Also, forgot to multiply by sizeof(Fetchi) in the memmove,
> synced flags properly on startup, even for existing messages, and
> fixed a misunderstanding about when I needed to adjust muid: we
> don't want to do it in expunge.
>
> The current revision of the patch (which I'm running now):
One more revision, which fixes a bug in the uid field we use for
the message. The bug was there from the start, but I added a field
that made it bite.
Thanks to piroko for debugging with me:
diff -r 8b2040ba4785 sys/src/cmd/upas/fs/imap.c
--- a/sys/src/cmd/upas/fs/imap.c Fri Nov 22 17:29:35 2019 +1030
+++ b/sys/src/cmd/upas/fs/imap.c Fri Nov 22 11:48:26 2019 -0800
@@ -35,6 +35,7 @@
uvlong uid;
ulong sizes;
ulong dates;
+ ulong flags;
} Fetchi;
typedef struct Imap Imap;
@@ -51,9 +52,23 @@
ulong tag;
ulong validity;
+ ulong newvalidity;
int nmsg;
int size;
+ /*
+ * These variables are how we keep track
+ * of what's been added or deleted. They
+ * keep a count of the number of uids we
+ * have processed this sync (nuid), and
+ * the number we processed last sync
+ * (muid).
+ *
+ * We keep the latest imap state in fetchi,
+ * and imap4read syncs the information in
+ * it with the messages. That's how we know
+ * something changed on the server.
+ */
Fetchi *f;
int nuid;
int muid;
@@ -163,20 +178,22 @@
Fetch,
Cap,
Auth,
+ Expunge,
Unknown,
};
static char *verblist[] = {
-[Ok] "ok",
-[No] "no",
-[Bad] "bad",
-[Bye] "bye",
-[Exists] "exists",
-[Status] "status",
-[Fetch] "fetch",
-[Cap] "capability",
-[Auth] "authenticate",
+ [Ok] "ok",
+ [No] "no",
+ [Bad] "bad",
+ [Bye] "bye",
+ [Exists] "exists",
+ [Status] "status",
+ [Fetch] "fetch",
+ [Cap] "capability",
+ [Auth] "authenticate",
+ [Expunge] "expunge",
};
static int
@@ -187,7 +204,7 @@
if(q = strchr(verb, ' '))
*q = '\0';
- for(i = 0; i < nelem(verblist) - 1; i++)
+ for(i = 0; i < nelem(verblist); i++)
if(strcmp(verblist[i], verb) == 0)
break;
if(q)
@@ -200,6 +217,7 @@
{
vlong v;
+ idprint(i, "mkuid: validity: %lud, idstr: '%s', val: %lud\n", i->validity, id, strtoul(id, 0, 10));
v = (vlong)i->validity<<32;
return v | strtoul(id, 0, 10);
}
@@ -230,26 +248,29 @@
"\\Stored", Fstored,
};
-static void
-parseflags(Message *m, char *s)
+static int
+parseflags(char *s)
{
char *f[10];
- int i, j, j0, n;
+ int i, j, j0, n, flg;
n = tokenize(s, f, nelem(f));
qsort(f, n, sizeof *f, (int (*)(void*,void*))strcmp);
j = 0;
- for(i = 0; i < n; i++)
+ flg = 0;
+ for(i = 0; i < n; i++){
for(j0 = j;; j++){
if(j == nelem(ftab)){
j = j0; /* restart search */
break;
}
- if(strcmp(f[i], ftab[j].flag) == 0){
- m->flags |= ftab[j].e;
+ if(cistrcmp(f[i], ftab[j].flag) == 0){
+ flg |= ftab[j].e;
break;
}
}
+ }
+ return flg;
}
/* "17-Jul-1996 02:44:25 -0700" */
@@ -311,7 +332,7 @@
int nargs;
for(nargs=0; nargs < maxargs; nargs++){
- while(*s!='\0' && utfrune(qsep, *s)!=nil)
+ while(*s!='\0' && utfrune(qsep, *s) != nil)
s++;
if(*s == '\0')
break;
@@ -323,7 +344,7 @@
}
static char*
-fetchrsp(Imap *imap, char *p, Mailbox *, Message *m)
+fetchrsp(Imap *imap, char *p, Mailbox *, Message *m, int idx)
{
char *f[15], *s, *q;
int i, n, a;
@@ -332,6 +353,11 @@
static char error[256];
extern void msgrealloc(Message*, ulong);
+ if(idx < 0 || idx >= imap->muid){
+ snprint(error, sizeof error, "fetchrsp: bad idx %d", idx);
+ return error;
+ }
+
redux:
n = imaptokenize(p, f, nelem(f));
if(n%2)
@@ -341,23 +367,26 @@
l = internaltounix(f[i + 1]);
if(l < 418319360)
abort();
- if(imap->nuid < imap->muid)
- imap->f[imap->nuid].dates = l;
+ if(idx < imap->muid)
+ imap->f[idx].dates = l;
}else if(strcmp(f[i], "rfc822.size") == 0){
l = strtoul(f[i + 1], 0, 0);
if(m)
m->size = l;
- else if(imap->nuid < imap->muid)
- imap->f[imap->nuid].sizes = l;
+ else if(idx < imap->muid)
+ imap->f[idx].sizes = l;
}else if(strcmp(f[i], "uid") == 0){
- v = mkuid(imap, f[1]);
+ v = mkuid(imap, f[i + 1]);
if(m)
m->imapuid = v;
- if(imap->nuid < imap->muid)
- imap->f[imap->nuid].uid = v;
+ if(idx < imap->muid)
+ imap->f[idx].uid = v;
}else if(strcmp(f[i], "flags") == 0){
+ l = parseflags(f[i + 1]);
if(m)
- parseflags(m, f[i + 1]);
+ m->flags = l;
+ if(idx < imap->muid)
+ imap->f[idx].flags = l;
}else if(strncmp(f[i], "body[]", 6) == 0){
s = f[i]+6;
o = 0;
@@ -396,7 +425,7 @@
}else
return confused;
}
- return 0;
+ return nil;
}
void
@@ -428,7 +457,7 @@
imap4resp0(Imap *imap, Mailbox *mb, Message *m)
{
char *e, *line, *p, *ep, *op, *q, *verb;
- int n, unexp;
+ int n, idx, unexp;
static char error[256];
unexp = 0;
@@ -484,7 +513,7 @@
if(q = strstr(p, "messages"))
imap->nmsg = strtoul(q + 8, 0, 10);
if(q = strstr(p, "uidvalidity"))
- imap->validity = strtoul(q + 11, 0, 10);
+ imap->newvalidity = strtoul(q + 11, 0, 10);
break;
case Fetch:
if(*p == '('){
@@ -492,9 +521,20 @@
if(ep[-1] == ')')
*--ep = 0;
}
- if(e = fetchrsp(imap, p, mb, m))
+ if(e = fetchrsp(imap, p, mb, m, n - 1))
eprint("imap: fetchrsp: %s\n", e);
- imap->nuid++;
+ if(n > 0 && n <= imap->muid && n > imap->nuid)
+ imap->nuid = n;
+ break;
+ case Expunge:
+ if(n < 1 || n > imap->muid){
+ snprint(error, sizeof(error), "bad expunge %d (nmsg %d)", n, imap->nuid);
+ return error;
+ }
+ idx = n - 1;
+ memmove(&imap->f[idx], &imap->f[idx + 1], (imap->nmsg - idx - 1)*sizeof(imap->f[0]));
+ imap->nmsg--;
+ imap->nuid--;
break;
case Auth:
break;
@@ -903,15 +943,30 @@
Fetchi *f;
Message *m, **ll;
+again:
imap4cmd(imap, "status %Z (messages uidvalidity)", imap->mbox);
if(!isokay(s = imap4resp(imap)))
return s;
+ /* the world shifted: start over */
+ if(imap->validity != imap->newvalidity){
+ imap->validity = imap->newvalidity;
+ imap->nuid = 0;
+ imap->muid = 0;
+ imap->nmsg = 0;
+ goto again;
+ }
- imap->nuid = 0;
+ imap->f = erealloc(imap->f, imap->nmsg*sizeof imap->f[0]);
+ if(imap->nmsg > imap->muid)
+ memset(&imap->f[imap->muid], 0, (imap->nmsg - imap->muid)*sizeof(imap->f[0]));
imap->muid = imap->nmsg;
- imap->f = erealloc(imap->f, imap->nmsg*sizeof imap->f[0]);
if(imap->nmsg > 0){
- imap4cmd(imap, "uid fetch 1:* (uid rfc822.size internaldate)");
+ n = imap->nuid;
+ if(n == 0)
+ n = 1;
+ if(n > imap->nmsg)
+ n = imap->nmsg;
+ imap4cmd(imap, "fetch %d:%d (uid flags rfc822.size internaldate)", n, imap->nmsg);
if(!isokay(s = imap4resp(imap)))
return s;
}
@@ -949,6 +1004,7 @@
m->imapuid = f[i].uid;
m->fileid = datesec(imap, i);
m->size = f[i].sizes;
+ m->flags = f[i].flags;
m->next = *ll;
*ll = m;
ll = &m->next;
@@ -960,6 +1016,8 @@
m->deleted = Disappear;
ll = &m->next;
}else{
+ /* TODO: flag this as changed, plumb. */
+ m->flags = f[i].flags;
ll = &m->next;
i++;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [9front] upas/fs imap fixes and improvements.
@ 2019-11-21 1:45 ori
0 siblings, 0 replies; 5+ messages in thread
From: ori @ 2019-11-21 1:45 UTC (permalink / raw)
To: cinap_lenrek, 9front
> on first glance:
>
> - if(imap->nuid < imap->muid)
> + if(idx < imap->muid)
> imap->f[imap->nuid].dates = l;
>
> should be: imap->f[idx].dates = l instead?
>
> + memcpy(&imap->f[idx], &imap->f[idx + 1], imap->nmsg - idx - 1);
>
> maybe use memmove() instead as source and dest overlap.
>
> --
> cinap
Fixed. Also, forgot to multiply by sizeof(Fetchi) in the memmove,
synced flags properly on startup, even for existing messages, and
fixed a misunderstanding about when I needed to adjust muid: we
don't want to do it in expunge.
The current revision of the patch (which I'm running now):
diff -r 25e81b900a8c sys/src/cmd/upas/fs/imap.c
--- a/sys/src/cmd/upas/fs/imap.c Wed Nov 20 16:12:21 2019 -0800
+++ b/sys/src/cmd/upas/fs/imap.c Wed Nov 20 17:43:54 2019 -0800
@@ -35,6 +35,7 @@
uvlong uid;
ulong sizes;
ulong dates;
+ ulong flags;
} Fetchi;
typedef struct Imap Imap;
@@ -51,9 +52,23 @@
ulong tag;
ulong validity;
+ ulong newvalidity;
int nmsg;
int size;
+ /*
+ * These variables are how we keep track
+ * of what's been added or deleted. They
+ * keep a count of the number of uids we
+ * have processed this sync (nuid), and
+ * the number we processed last sync
+ * (muid).
+ *
+ * We keep the latest imap state in fetchi,
+ * and imap4read syncs the information in
+ * it with the messages. That's how we know
+ * something changed on the server.
+ */
Fetchi *f;
int nuid;
int muid;
@@ -163,20 +178,22 @@
Fetch,
Cap,
Auth,
+ Expunge,
Unknown,
};
static char *verblist[] = {
-[Ok] "ok",
-[No] "no",
-[Bad] "bad",
-[Bye] "bye",
-[Exists] "exists",
-[Status] "status",
-[Fetch] "fetch",
-[Cap] "capability",
-[Auth] "authenticate",
+ [Ok] "ok",
+ [No] "no",
+ [Bad] "bad",
+ [Bye] "bye",
+ [Exists] "exists",
+ [Status] "status",
+ [Fetch] "fetch",
+ [Cap] "capability",
+ [Auth] "authenticate",
+ [Expunge] "expunge",
};
static int
@@ -187,7 +204,7 @@
if(q = strchr(verb, ' '))
*q = '\0';
- for(i = 0; i < nelem(verblist) - 1; i++)
+ for(i = 0; i < nelem(verblist); i++)
if(strcmp(verblist[i], verb) == 0)
break;
if(q)
@@ -230,26 +247,29 @@
"\\Stored", Fstored,
};
-static void
-parseflags(Message *m, char *s)
+static int
+parseflags(char *s)
{
char *f[10];
- int i, j, j0, n;
+ int i, j, j0, n, flg;
n = tokenize(s, f, nelem(f));
qsort(f, n, sizeof *f, (int (*)(void*,void*))strcmp);
j = 0;
- for(i = 0; i < n; i++)
+ flg = 0;
+ for(i = 0; i < n; i++){
for(j0 = j;; j++){
if(j == nelem(ftab)){
j = j0; /* restart search */
break;
}
- if(strcmp(f[i], ftab[j].flag) == 0){
- m->flags |= ftab[j].e;
+ if(cistrcmp(f[i], ftab[j].flag) == 0){
+ flg |= ftab[j].e;
break;
}
}
+ }
+ return flg;
}
/* "17-Jul-1996 02:44:25 -0700" */
@@ -311,7 +331,7 @@
int nargs;
for(nargs=0; nargs < maxargs; nargs++){
- while(*s!='\0' && utfrune(qsep, *s)!=nil)
+ while(*s!='\0' && utfrune(qsep, *s) != nil)
s++;
if(*s == '\0')
break;
@@ -323,7 +343,7 @@
}
static char*
-fetchrsp(Imap *imap, char *p, Mailbox *, Message *m)
+fetchrsp(Imap *imap, char *p, Mailbox *, Message *m, int idx)
{
char *f[15], *s, *q;
int i, n, a;
@@ -332,6 +352,11 @@
static char error[256];
extern void msgrealloc(Message*, ulong);
+ if(idx < 0 || idx >= imap->muid){
+ snprint(error, sizeof error, "fetchrsp: bad idx %d", idx);
+ return error;
+ }
+
redux:
n = imaptokenize(p, f, nelem(f));
if(n%2)
@@ -341,23 +366,26 @@
l = internaltounix(f[i + 1]);
if(l < 418319360)
abort();
- if(imap->nuid < imap->muid)
- imap->f[imap->nuid].dates = l;
+ if(idx < imap->muid)
+ imap->f[idx].dates = l;
}else if(strcmp(f[i], "rfc822.size") == 0){
l = strtoul(f[i + 1], 0, 0);
if(m)
m->size = l;
- else if(imap->nuid < imap->muid)
- imap->f[imap->nuid].sizes = l;
+ else if(idx < imap->muid)
+ imap->f[idx].sizes = l;
}else if(strcmp(f[i], "uid") == 0){
v = mkuid(imap, f[1]);
if(m)
m->imapuid = v;
- if(imap->nuid < imap->muid)
- imap->f[imap->nuid].uid = v;
+ if(idx < imap->muid)
+ imap->f[idx].uid = v;
}else if(strcmp(f[i], "flags") == 0){
+ l = parseflags(f[i + 1]);
if(m)
- parseflags(m, f[i + 1]);
+ m->flags = l;
+ if(idx < imap->muid)
+ imap->f[idx].flags = l;
}else if(strncmp(f[i], "body[]", 6) == 0){
s = f[i]+6;
o = 0;
@@ -396,7 +424,7 @@
}else
return confused;
}
- return 0;
+ return nil;
}
void
@@ -428,7 +456,7 @@
imap4resp0(Imap *imap, Mailbox *mb, Message *m)
{
char *e, *line, *p, *ep, *op, *q, *verb;
- int n, unexp;
+ int i, n, idx, unexp;
static char error[256];
unexp = 0;
@@ -484,7 +512,7 @@
if(q = strstr(p, "messages"))
imap->nmsg = strtoul(q + 8, 0, 10);
if(q = strstr(p, "uidvalidity"))
- imap->validity = strtoul(q + 11, 0, 10);
+ imap->newvalidity = strtoul(q + 11, 0, 10);
break;
case Fetch:
if(*p == '('){
@@ -492,9 +520,27 @@
if(ep[-1] == ')')
*--ep = 0;
}
- if(e = fetchrsp(imap, p, mb, m))
+ if(e = fetchrsp(imap, p, mb, m, n - 1))
eprint("imap: fetchrsp: %s\n", e);
- imap->nuid++;
+ idprint(imap, "n = %d, muid = %d, nuid = %d\n", n, imap->muid, imap->nuid);
+ if(n > 0 && n <= imap->muid && n > imap->nuid)
+ imap->nuid = n;
+ break;
+ case Expunge:
+ if(n < 1 || n > imap->muid){
+ snprint(error, sizeof(error), "bad expunge %d (nmsg %d)", n, imap->nuid);
+ return error;
+ }
+ idx = n - 1;
+ print("before:\n");
+ for(i = 0; i < imap->nmsg; i++)
+ print(" %d: %U\n", i, imap->f[i].uid);
+ memmove(&imap->f[idx], &imap->f[idx + 1], (imap->nmsg - idx - 1)*sizeof(imap->f[0]));
+ imap->nmsg--;
+ imap->nuid--;
+ print("after:\n");
+ for(i = 0; i < imap->nmsg; i++)
+ print(" %d: %U\n", i, imap->f[i].uid);
break;
case Auth:
break;
@@ -903,15 +949,30 @@
Fetchi *f;
Message *m, **ll;
+again:
imap4cmd(imap, "status %Z (messages uidvalidity)", imap->mbox);
if(!isokay(s = imap4resp(imap)))
return s;
+ /* the world shifted: start over */
+ if(imap->validity != imap->newvalidity){
+ imap->validity = imap->newvalidity;
+ imap->nuid = 0;
+ imap->muid = 0;
+ imap->nmsg = 0;
+ goto again;
+ }
- imap->nuid = 0;
+ imap->f = erealloc(imap->f, imap->nmsg*sizeof imap->f[0]);
+ if(imap->nmsg > imap->muid)
+ memset(&imap->f[imap->muid], 0, (imap->nmsg - imap->muid)*sizeof(imap->f[0]));
imap->muid = imap->nmsg;
- imap->f = erealloc(imap->f, imap->nmsg*sizeof imap->f[0]);
if(imap->nmsg > 0){
- imap4cmd(imap, "uid fetch 1:* (uid rfc822.size internaldate)");
+ n = imap->nuid;
+ if(n == 0)
+ n = 1;
+ if(n > imap->nmsg)
+ n = imap->nmsg;
+ imap4cmd(imap, "fetch %d:%d (uid flags rfc822.size internaldate)", n, imap->nmsg);
if(!isokay(s = imap4resp(imap)))
return s;
}
@@ -949,6 +1010,7 @@
m->imapuid = f[i].uid;
m->fileid = datesec(imap, i);
m->size = f[i].sizes;
+ m->flags = f[i].flags;
m->next = *ll;
*ll = m;
ll = &m->next;
@@ -960,6 +1022,8 @@
m->deleted = Disappear;
ll = &m->next;
}else{
+ /* TODO: plumb changed flags. */
+ m->flags = f[i].flags;
ll = &m->next;
i++;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-11-22 23:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-20 7:34 [9front] upas/fs imap fixes and improvements cinap_lenrek
2019-11-21 1:45 ori
2019-11-22 19:49 ori
2019-11-22 20:50 cinap_lenrek
2019-11-22 23:46 ori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).