Re: deleted DELETE
On Wed, 29 Aug 2007, Ken Murchison wrote: Small bug fix (the p++ line) attached. Good catch. I didn't look at the cyr_expire patch real close, since I assumed Fastmail was using it in production. I know, I know, I shouldn't assume anything and test it myself. Mea culpa. Well, if Bron can spot bugs and blatant omissions in my patches and I can spot bugs in his code, then peer review seems to be working. It would probably work faster if we were all running Cyrus CVS rather than our own private source trees. I hope to get there about this time next year. There shouldn't be any embarrassment about broken code in CVS, so long as we are able to generate a separate branch for emergency bug fix releases. At the end of the day CMU's CVS repository is the canonical reference. Trying to track third party patches (and patches to patches) is hard work. -- David Carter Email: [EMAIL PROTECTED] University Computing Service,Phone: (01223) 334502 New Museums Site, Pembroke Street, Fax: (01223) 334679 Cambridge UK. CB2 3QH.
Re: deleted DELETE
On Wed, 29 Aug 2007, David Carter wrote:
mboxname_isusermailbox() works on internal mailbox names. I don't think that
it needs to worry about IMAPOPT_UNIXHIERARCHYSEP.
Here's a trivial patch to remove the IMAPOPT_UNIXHIERARCHYSEP stuff.
Otherwise mboxname_isusermailbox() is broken when unixhierarchysep is set.
--
David Carter Email: [EMAIL PROTECTED]
University Computing Service,Phone: (01223) 334502
New Museums Site, Pembroke Street, Fax: (01223) 334679
Cambridge UK. CB2 3QH.
Index: imap/mboxname.c
===
RCS file: /cvs/src/cyrus/imap/mboxname.c,v
retrieving revision 1.38
diff -u -d -r1.38 mboxname.c
--- imap/mboxname.c 28 Aug 2007 18:42:28 - 1.38
+++ imap/mboxname.c 30 Aug 2007 09:25:27 -
@@ -601,7 +601,6 @@
const char *p;
const char *start = name;
const char *deletedprefix = config_getstring(IMAPOPT_DELETEDPREFIX);
-const char sep = config_getswitch(IMAPOPT_UNIXHIERARCHYSEP) ? '/' : '.';
int len = strlen(deletedprefix);
int isdel = 0;
@@ -610,13 +609,13 @@
start = p + 1;
/* step past any deleted bit */
-if (mboxlist_delayed_delete_isenabled() strlen(start) len !strncmp(start,
deletedprefix, len) start[len] == sep) {
+if (mboxlist_delayed_delete_isenabled() strlen(start) len !strncmp(start,
deletedprefix, len) start[len] == '.') {
start += len + 1;
- isdel = 1; /* there's an additional sep + hextimestamp on isdel folders
*/
+ isdel = 1; /* there's an additional '.' + hextimestamp on isdel folders
*/
}
-if (strlen(start) 5 !strncmp(start, user, 4) start[4] == sep
- (!isinbox || !strchr(start+5, sep)) || (isdel (p = strchr(start+5, sep)) !strchr(p+1, sep)))
+if (strlen(start) 5 !strncmp(start, user, 4) start[4] == '.'
+ (!isinbox || !strchr(start+5, '.')) || (isdel (p = strchr(start+5, '.')) !strchr(p+1, '.')))
return (char*) start+5;
else
return NULL;
Re: Need advice: mailbox-based \Seen flag
Ken Murchison wrote: Boris Lytochkin wrote: Hello! I need an advice in implementing per-mailbox \Seen flag (or 'shared' \Seen flag in per-user basis). For now I want to implement it this way: 1) add a new mailbox attribute, say 'sharedseen' 2) switch path for .seen file in imap/seen_*.c:seen_getpath() if 'sharedseen' flag is set on mailbox. 3) changes in replication module? So, setting 'sharedseen' attribute to mailbox will cause using per-mailbox seen-file. What are weak points of this implementation? Is there more correct way to make per-mailbox \Seen flag? My quick thoughts would be the following: - Add a 'sharedseen' mailbox attribute as you suggest (as a mailbox annotation) - When a mailbox is opened that has this annotation enabled, we open the seen state database for the special 'anyone' user instead of the authorized user. This would allow using the same API with minimal changes. Perhaps we could come up with a better userid then 'anyone' but its already reserved as special via ACL. - - As an alternative to using a seen db for 'anyone' we could go back to using a local seen db in the mailbox folder, but then we'd have to have function pointers to swap between the functions for local access vs per-user access. - As another alternative, we could keep track of shared seen state in cyrus.index, but that would require an upgrade to the index file format, and more API changes. I actually went digging into seen state and cyrus.index a little today for another reason, and I realized that we wouldn't have to expand/upgrade the index file to accomodate a shared seen state. The existing field for system flags is just a bitmask, so we could easily add a bit for shared \Seen. We could also implement the sharedseen annotation as a mailbox flag in the index header, just like condstore. Based on other changes that I'm thinking about, I'm starting to like this idea better, even thought it *may* involve more code. -- Kenneth Murchison Systems Programmer Project Cyrus Developer/Maintainer Carnegie Mellon University
Re: Need advice: mailbox-based \Seen flag
I actually went digging into seen state and cyrus.index a little today for another reason, and I realized that we wouldn't have to expand/upgrade the index file to accomodate a shared seen state. The existing field for system flags is just a bitmask, so we could easily add a bit for shared \Seen. We could also implement the sharedseen annotation as a mailbox flag in the index header, just like condstore. Based on other changes that I'm thinking about, I'm starting to like this idea better, even thought it *may* involve more code. Doesn't that just seem annoying having two *completely* different ways of storing seen state? There' probably quite a few places in the code that look for the seen state. It just seems much easier to me to have a single flag that controls whether to open a specific users seen db, or the anyone user seen db, rather than having to special case lots of code around the place for either pulling from the seen db, or looking up a bit in the index file. Rob
Re: Need advice: mailbox-based \Seen flag
Rob Mueller wrote: I actually went digging into seen state and cyrus.index a little today for another reason, and I realized that we wouldn't have to expand/upgrade the index file to accomodate a shared seen state. The existing field for system flags is just a bitmask, so we could easily add a bit for shared \Seen. We could also implement the sharedseen annotation as a mailbox flag in the index header, just like condstore. Based on other changes that I'm thinking about, I'm starting to like this idea better, even thought it *may* involve more code. Doesn't that just seem annoying having two *completely* different ways of storing seen state? There' probably quite a few places in the code that look for the seen state. It just seems much easier to me to have a single flag that controls whether to open a specific users seen db, or the anyone user seen db, rather than having to special case lots of code around the place for either pulling from the seen db, or looking up a bit in the index file. Yeah, I talked myself out of this shortly after I sent the message. -- Kenneth Murchison Systems Programmer Project Cyrus Developer/Maintainer Carnegie Mellon University
Re: Need advice: mailbox-based \Seen flag
Boris Lytochkin wrote: My quick thoughts would be the following: - Add a 'sharedseen' mailbox attribute as you suggest (as a mailbox annotation) - When a mailbox is opened that has this annotation enabled, we open the seen state database for the special 'anyone' user instead of the authorized user. This would allow using the same API with minimal changes. Perhaps we could come up with a better userid then 'anyone' but its already reserved as special via ACL. It will work as long as we want to use shared folder for every user. As soon as we want to restrict access to a set of users, this would not work. That's what the 's' right is for. Only give 's' to those users that you want to be able to mark messages as seen. Let me explain why I need this. There is an account that gets all root and robot mail. So shared \Seen would have meaning of 'this mail has been read by admins'. That is why I need to restrict access to these shared folders. -- Kenneth Murchison Systems Programmer Project Cyrus Developer/Maintainer Carnegie Mellon University
