Check the uid string is all digits before trying to write a 'standard'

2000-11-16  Not Zed  <NotZed@HelixCode.com>

        * providers/local/camel-local-summary.c
        (local_summary_encode_x_evolution): Check the uid string is all
        digits before trying to write a 'standard' x-ev header.

        * providers/local/camel-maildir-summary.c
        (camel_maildir_summary_info_to_name): Convert an info into a
        maildir name:info filename.
        (camel_maildir_summary_name_to_info): Convert a name:info filename
        into an info, and tell us if it didn't match it.
        (message_info_new): When creating a new filename, gets its info
        from the flags field.  Likewise if creating from an existing file,
        extract the flags.
        (maildir_summary_sync): Remove a small memleak.  Also, if our
        flags and that requires a filename change, perform that here.
        (message_info_new): Get the received date from the filename.
        Also, dont overwirte the uid if we have one.
        (maildir_summary_check): Sort the summary in received order before
        completion.
        (maildir_summary_next_uid_string): Test the name for collusions
        before we give it out.  Retry, and if that fails, well, I guess we
        collide :(

        * providers/local/camel-mbox-folder.c (mbox_lock): Implement mbox
        locking.
        (mbox_unlock): And unlocking.
        (mbox_append_message): Lock the folder for write before doing
        anything.
        (mbox_get_message): Lock the folder for read before doing
        anything.

        * providers/local/camel-local-folder.c (camel_local_folder_lock):
        Implement something here.  We handle the recursive ability but
        pass the locking to the folder itself.
        (camel_local_folder_unlock): Likewise for unlocking.
        (local_lock): Default - do nothing, return success.
        (local_unlock): Same.
        (local_sync): Changed slightly for locking api changes, and also,
        only lock around the sync process itself.

        * camel-lock.c: New file - utility functions for locking using
        different strategies and/or for locking folders safely.

        * Makefile.am (libcamel_la_SOURCES): Adde camel-lock.[ch]

svn path=/trunk/; revision=6592

9 files changed, 307 insertions, 230 deletions

diff --git a/camel/providers/local/camel-local-folder.c b/camel/providers/local/camel-local-folder.c
index 034f91d7ee..bec2d7fb56 100644
--- a/camel/providers/local/camel-local-folder.c
+++ b/camel/providers/local/camel-local-folder.c
@@ -1,7 +1,5 @@
-/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8; fill-column: 160 -*- */
-/* camel-local-folder.c : Abstract class for an email folder */
-
-/* 
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8; fill-column: 160 -*-
+ * 
  * Authors: Michael Zucchi <notzed@helixcode.com>
  *
  * Copyright (C) 1999, 2000 Helix Code Inc.
@@ -53,6 +51,8 @@ static CamelFolderClass *parent_class = NULL;
 #define CF_CLASS(so) CAMEL_FOLDER_CLASS (CAMEL_OBJECT_GET_CLASS(so))
 #define CLOCALS_CLASS(so) CAMEL_STORE_CLASS (CAMEL_OBJECT_GET_CLASS(so))
 
+static int local_lock(CamelLocalFolder *lf, CamelLockType type, CamelException *ex);
+static void local_unlock(CamelLocalFolder *lf);
 
 static void local_sync(CamelFolder *folder, gboolean expunge, CamelException *ex);
 static gint local_get_message_count(CamelFolder *folder);
@@ -111,6 +111,9 @@ camel_local_folder_class_init(CamelLocalFolderClass * camel_local_folder_class)
 	camel_folder_class->set_message_user_flag = local_set_message_user_flag;
 	camel_folder_class->get_message_user_tag = local_get_message_user_tag;
 	camel_folder_class->set_message_user_tag = local_set_message_user_tag;
+
+	camel_local_folder_class->lock = local_lock;
+	camel_local_folder_class->unlock = local_unlock;
 }
 
 static void
@@ -221,36 +224,66 @@ camel_local_folder_construct(CamelLocalFolder *lf, CamelStore *parent_store, con
 	return lf;
 }
 
-/* Have to work out how/when to lock */
-int camel_local_folder_lock(CamelLocalFolder *lf, CamelException *ex)
+/* lock the folder, may be called repeatedly (with matching unlock calls),
+   with type the same or less than the first call */
+int camel_local_folder_lock(CamelLocalFolder *lf, CamelLockType type, CamelException *ex)
+{
+	if (lf->locked > 0) {
+		/* lets be anal here - its important the code knows what its doing */
+		g_assert(lf->locktype == type || lf->locktype == CAMEL_LOCK_WRITE);
+	} else {
+		if (CLOCALF_CLASS(lf)->lock(lf, type, ex) == -1)
+			return -1;
+		lf->locktype = type;
+	}
+
+	lf->locked++;
+
+	return 0;
+}
+
+/* unlock folder */
+int camel_local_folder_unlock(CamelLocalFolder *lf)
 {
+	g_assert(lf->locked>0);
+	lf->locked--;
+	if (lf->locked == 0)
+		CLOCALF_CLASS(lf)->unlock(lf);
+
 	return 0;
 }
 
-int camel_local_folder_unlock(CamelLocalFolder *lf, CamelException *ex)
+static int
+local_lock(CamelLocalFolder *lf, CamelLockType type, CamelException *ex)
 {
 	return 0;
 }
 
 static void
+local_unlock(CamelLocalFolder *lf)
+{
+	/* nothing */
+}
+
+static void
 local_sync(CamelFolder *folder, gboolean expunge, CamelException *ex)
 {
 	CamelLocalFolder *lf = CAMEL_LOCAL_FOLDER(folder);
 
 	d(printf("local sync, expunge=%s\n", expunge?"true":"false"));
 
-	if (camel_local_folder_lock(lf, ex) == -1)
+	if (camel_local_folder_lock(lf, CAMEL_LOCK_WRITE, ex) == -1)
 		return;
 
+	/* if sync fails, we'll pass it up on exit through ex */
 	camel_local_summary_sync(lf->summary, expunge, lf->changes, ex);
+	camel_local_folder_unlock(lf);
+
 	if (camel_folder_change_info_changed(lf->changes)) {
 		camel_object_trigger_event(CAMEL_OBJECT(folder), "folder_changed", lf->changes);
 		camel_folder_change_info_clear(lf->changes);
 	}
 
-	if (camel_local_folder_unlock(lf, ex) == -1)
-		return;
-
 	/* force save of metadata */
 	if (lf->index)
 		ibex_save(lf->index);
@@ -267,186 +300,6 @@ local_expunge(CamelFolder *folder, CamelException *ex)
 	camel_folder_sync(folder, TRUE, ex);
 }
 
-#if 0
-static void
-local_append_message(CamelFolder *folder, CamelMimeMessage * message, const CamelMessageInfo * info, CamelException *ex)
-{
-	CamelLocalFolder *local_folder = CAMEL_LOCAL_FOLDER(folder);
-	CamelStream *output_stream = NULL, *filter_stream = NULL;
-	CamelMimeFilter *filter_from = NULL;
-	CamelMessageInfo *newinfo;
-	struct stat st;
-	off_t seek = -1;
-	char *xev;
-	guint32 uid;
-	char *fromline = NULL;
-
-	if (stat(local_folder->folder_path, &st) != 0)
-		goto fail;
-
-	output_stream = camel_stream_fs_new_with_name(local_folder->folder_file_path, O_WRONLY|O_APPEND, 0600);
-	if (output_stream == NULL)
-		goto fail;
-
-	seek = st.st_size;
-
-	/* assign a new x-evolution header/uid */
-	camel_medium_remove_header(CAMEL_MEDIUM(message), "X-Evolution");
-	uid = camel_folder_summary_next_uid(CAMEL_FOLDER_SUMMARY(local_folder->summary));
-	/* important that the header matches exactly 00000000-0000 */
-	xev = g_strdup_printf("%08x-%04x", uid, info ? info->flags & 0xFFFF : 0);
-	camel_medium_add_header(CAMEL_MEDIUM(message), "X-Evolution", xev);
-	g_free(xev);
-
-	/* we must write this to the non-filtered stream ... */
-	fromline = camel_local_summary_build_from(CAMEL_MIME_PART(message)->headers);
-	if (camel_stream_printf(output_stream, seek==0?"%s":"\n%s", fromline) == -1)
-		goto fail;
-
-	/* and write the content to the filtering stream, that translated '\nFrom' into '\n>From' */
-	filter_stream = (CamelStream *) camel_stream_filter_new_with_stream(output_stream);
-	filter_from = (CamelMimeFilter *) camel_mime_filter_from_new();
-	camel_stream_filter_add((CamelStreamFilter *) filter_stream, filter_from);
-	if (camel_data_wrapper_write_to_stream(CAMEL_DATA_WRAPPER(message), filter_stream) == -1)
-		goto fail;
-
-	if (camel_stream_close(filter_stream) == -1)
-		goto fail;
-
-	/* filter stream ref's the output stream itself, so we need to unref it too */
-	camel_object_unref(CAMEL_OBJECT(filter_from));
-	camel_object_unref(CAMEL_OBJECT(filter_stream));
-	camel_object_unref(CAMEL_OBJECT(output_stream));
-	g_free(fromline);
-
-	/* force a summary update - will only update from the new position, if it can */
-	if (camel_local_summary_update(local_folder->summary, seek==0?seek:seek+1, local_folder->changes) == 0) {
-		char uidstr[16];
-
-		sprintf(uidstr, "%u", uid);
-		newinfo = camel_folder_summary_uid(CAMEL_FOLDER_SUMMARY(local_folder->summary), uidstr);
-
-		if (info && newinfo) {
-			CamelFlag *flag = info->user_flags;
-			CamelTag *tag = info->user_tags;
-
-			while (flag) {
-				camel_flag_set(&(newinfo->user_flags), flag->name, TRUE);
-				flag = flag->next;
-			}
-
-			while (tag) {
-				camel_tag_set(&(newinfo->user_tags), tag->name, tag->value);
-				tag = tag->next;
-			}
-		}
-		camel_object_trigger_event(CAMEL_OBJECT(folder), "folder_changed", local_folder->changes);
-		camel_folder_change_info_clear(local_folder->changes);
-	}
-
-	return;
-
-      fail:
-	if (camel_exception_is_set(ex)) {
-		camel_exception_setv(ex, camel_exception_get_id(ex),
-				     _("Cannot append message to local file: %s"), camel_exception_get_description(ex));
-	} else {
-		camel_exception_setv(ex, CAMEL_EXCEPTION_SYSTEM,
-				     _("Cannot append message to local file: %s"), g_strerror(errno));
-	}
-	if (filter_stream) {
-		/*camel_stream_close (filter_stream); */
-		camel_object_unref(CAMEL_OBJECT(filter_stream));
-	}
-	if (output_stream)
-		camel_object_unref(CAMEL_OBJECT(output_stream));
-
-	if (filter_from)
-		camel_object_unref(CAMEL_OBJECT(filter_from));
-
-	g_free(fromline);
-
-	/* make sure the file isn't munged by us */
-	if (seek != -1) {
-		int fd = open(local_folder->folder_file_path, O_WRONLY, 0600);
-
-		if (fd != -1) {
-			ftruncate(fd, st.st_size);
-			close(fd);
-		}
-	}
-}
-
-static CamelMimeMessage *
-local_get_message(CamelFolder *folder, const gchar * uid, CamelException *ex)
-{
-	CamelLocalFolder *local_folder = CAMEL_LOCAL_FOLDER(folder);
-	CamelStream *message_stream = NULL;
-	CamelMimeMessage *message = NULL;
-	CamelLocalMessageInfo *info;
-	CamelMimeParser *parser = NULL;
-	char *buffer;
-	int len;
-
-	/* get the message summary info */
-	info = (CamelLocalMessageInfo *) camel_folder_summary_uid(CAMEL_FOLDER_SUMMARY(local_folder->summary), uid);
-
-	if (info == NULL) {
-		errno = ENOENT;
-		goto fail;
-	}
-
-	/* if this has no content, its an error in the library */
-	g_assert(info->info.content);
-	g_assert(info->frompos != -1);
-
-	/* where we read from */
-	message_stream = camel_stream_fs_new_with_name(local_folder->folder_file_path, O_RDONLY, 0);
-	if (message_stream == NULL)
-		goto fail;
-
-	/* we use a parser to verify the message is correct, and in the correct position */
-	parser = camel_mime_parser_new();
-	camel_mime_parser_init_with_stream(parser, message_stream);
-	camel_object_unref(CAMEL_OBJECT(message_stream));
-	camel_mime_parser_scan_from(parser, TRUE);
-
-	camel_mime_parser_seek(parser, info->frompos, SEEK_SET);
-	if (camel_mime_parser_step(parser, &buffer, &len) != HSCAN_FROM) {
-		g_warning("File appears truncated");
-		goto fail;
-	}
-
-	if (camel_mime_parser_tell_start_from(parser) != info->frompos) {
-		/* TODO: This should probably perform a re-sync/etc, and try again? */
-		g_warning("Summary doesn't match the folder contents!  eek!\n"
-			  "  expecting offset %ld got %ld", (long int)info->frompos,
-			  (long int)camel_mime_parser_tell_start_from(parser));
-		errno = EINVAL;
-		goto fail;
-	}
-
-	message = camel_mime_message_new();
-	if (camel_mime_part_construct_from_parser(CAMEL_MIME_PART(message), parser) == -1) {
-		g_warning("Construction failed");
-		goto fail;
-	}
-	camel_object_unref(CAMEL_OBJECT(parser));
-
-	return message;
-
-      fail:
-	camel_exception_setv(ex, CAMEL_EXCEPTION_FOLDER_INVALID_UID, _("Cannot get message: %s"), g_strerror(errno));
-
-	if (parser)
-		camel_object_unref(CAMEL_OBJECT(parser));
-	if (message)
-		camel_object_unref(CAMEL_OBJECT(message));
-
-	return NULL;
-}
-#endif
-
 /*
   The following functions all work off the summary, so the default operations provided
   in camel-local-folder will suffice for all subclasses.  They may want to
diff --git a/camel/providers/local/camel-local-folder.h b/camel/providers/local/camel-local-folder.h
index fa0ad34e84..914f996f9a 100644
--- a/camel/providers/local/camel-local-folder.h
+++ b/camel/providers/local/camel-local-folder.h
@@ -32,6 +32,7 @@ extern "C" {
 #include <camel/camel-folder-search.h>
 #include <libibex/ibex.h>
 #include "camel-local-summary.h"
+#include "camel-lock.h"
 
 /*  #include "camel-store.h" */
 
@@ -46,6 +47,7 @@ typedef struct {
 	guint32 flags;		/* open mode flags */
 
 	int locked;		/* lock counter */
+	CamelLockType locktype;	/* what type of lock we have */
 
 	char *base_path;	/* base path of the local folder */
 	char *folder_path;	/* the path to the folder itself */
@@ -67,10 +69,10 @@ typedef struct {
 	CamelLocalSummary *(*create_summary)(const char *path, const char *folder, ibex *index);
 
 	/* Lock the folder for my operations */
-	int (*lock)(CamelLocalFolder *);
+	int (*lock)(CamelLocalFolder *, CamelLockType type, CamelException *ex);
 
 	/* Unlock the folder for my operations */
-	int (*unlock)(CamelLocalFolder *);
+	void (*unlock)(CamelLocalFolder *);
 } CamelLocalFolderClass;
 
 
@@ -84,8 +86,8 @@ CamelType camel_local_folder_get_type(void);
 
 /* Lock the folder for internal use.  May be called repeatedly */
 /* UNIMPLEMENTED */
-int camel_local_folder_lock(CamelLocalFolder *lf, CamelException *ex);
-int camel_local_folder_unlock(CamelLocalFolder *lf, CamelException *ex);
+int camel_local_folder_lock(CamelLocalFolder *lf, CamelLockType type, CamelException *ex);
+int camel_local_folder_unlock(CamelLocalFolder *lf);
 
 #ifdef __cplusplus
 }
diff --git a/camel/providers/local/camel-local-summary.c b/camel/providers/local/camel-local-summary.c
index ed33d9ffc2..7455e96cf8 100644
--- a/camel/providers/local/camel-local-summary.c
+++ b/camel/providers/local/camel-local-summary.c
@@ -290,12 +290,15 @@ local_summary_encode_x_evolution(CamelLocalSummary *cls, const CamelMessageInfo
 	GString *val = g_string_new("");
 	CamelFlag *flag = mi->user_flags;
 	CamelTag *tag = mi->user_tags;
-	char *ret;
+	char *ret, *p;
 	guint32 uid;
 
 	/* FIXME: work out what to do with uid's that aren't stored here? */
 	/* FIXME: perhaps make that a mbox folder only issue?? */
-	if (sscanf(mi->uid, "%u", &uid) == 1) {
+	p = mi->uid;
+	while (*p && isdigit(*p))
+		p++;
+	if (*p == 0 && sscanf(mi->uid, "%u", &uid) == 1) {
 		g_string_sprintf(out, "%08x-%04x", uid, mi->flags & 0xffff);
 	} else {
 		g_string_sprintf(out, "%s-%04x", mi->uid, mi->flags & 0xffff);
diff --git a/camel/providers/local/camel-maildir-folder.c b/camel/providers/local/camel-maildir-folder.c
index 0cf3f23610..e3ff088238 100644
--- a/camel/providers/local/camel-maildir-folder.c
+++ b/camel/providers/local/camel-maildir-folder.c
@@ -96,7 +96,7 @@ CamelType camel_maildir_folder_get_type(void)
 							   (CamelObjectInitFunc) maildir_init,
 							   (CamelObjectFinalizeFunc) maildir_finalize);
 	}
-
+ 
 	return camel_maildir_folder_type;
 }
 
@@ -128,8 +128,6 @@ static void maildir_append_message(CamelFolder * folder, CamelMimeMessage * mess
 	CamelMaildirMessageInfo *mdi;
 	char *name, *dest;
 
-	/* FIXME: probably needs additional locking */
-
 	d(printf("Appending message\n"));
 
 	/* add it to the summary/assign the uid, etc */
@@ -202,6 +200,7 @@ static CamelMimeMessage *maildir_get_message(CamelFolder * folder, const gchar *
 
 	mdi = (CamelMaildirMessageInfo *)info;
 
+	/* what do we do if the message flags (and :info data) changes?  filename mismatch - need to recheck I guess */
 	name = g_strdup_printf("%s/cur/%s", lf->folder_path, mdi->filename);
 	if ((message_stream = camel_stream_fs_new_with_name(name, O_RDONLY, 0)) == NULL) {
 		camel_exception_setv(ex, CAMEL_EXCEPTION_FOLDER_INVALID_UID, _("Cannot get message: %s\n  %s"),
diff --git a/camel/providers/local/camel-maildir-summary.c b/camel/providers/local/camel-maildir-summary.c
index cbe5a56f00..92b2be23f8 100644
--- a/camel/providers/local/camel-maildir-summary.c
+++ b/camel/providers/local/camel-maildir-summary.c
@@ -133,6 +133,71 @@ CamelMaildirSummary	*camel_maildir_summary_new	(const char *filename, const char
 	return o;
 }
 
+/* the 'standard' maildir flags.  should be defined in sorted order. */
+static struct {
+	char flag;
+	guint32 flagbit;
+} flagbits[] = {
+	{ 'F', CAMEL_MESSAGE_FLAGGED },
+	{ 'R', CAMEL_MESSAGE_ANSWERED },
+	{ 'S', CAMEL_MESSAGE_SEEN },
+	{ 'T', CAMEL_MESSAGE_DELETED },
+};
+
+/* convert the uid + flags into a unique:info maildir format */
+char *camel_maildir_summary_info_to_name(const CamelMessageInfo *info)
+{
+	char *p, *buf;
+	int i;
+
+	buf = alloca(strlen(info->uid) + strlen(":2,") +  (sizeof(flagbits)/sizeof(flagbits[0])) + 1);
+	p = buf + sprintf(buf, "%s:2,", info->uid);
+	for (i=0;i<sizeof(flagbits)/sizeof(flagbits[0]);i++) {
+		if (info->flags & flagbits[i].flagbit)
+			*p++ = flagbits[i].flag;
+	}
+	*p = 0;
+
+	return g_strdup(buf);
+}
+
+/* returns 0 if the info matches (or there was none), otherwise we changed it */
+int camel_maildir_summary_name_to_info(CamelMessageInfo *info, const char *name)
+{
+	char *p, c;
+	guint32 set = 0;	/* what we set */
+	/*guint32 all = 0;*/	/* all flags */
+	int i;
+
+	p = strstr(name, ":2,");
+	if (p) {
+		p+=3;
+		while ((c = *p++)) {
+			/* we could assume that the flags are in order, but its just as easy not to require */
+			for (i=0;i<sizeof(flagbits)/sizeof(flagbits[0]);i++) {
+				if (flagbits[i].flag == c && (info->flags & flagbits[i].flagbit) == 0) {
+					set |= flagbits[i].flagbit;
+				}
+				/*all |= flagbits[i].flagbit;*/
+			}
+		}
+
+		/* changed? */
+		/*if ((info->flags & all) != set) {*/
+		if ((info->flags & set) != set) {
+			/* ok, they did change, only add the new flags ('merge flags'?) */
+			/*info->flags &= all;  if we wanted to set only the new flags, which we probably dont */
+			info->flags |= set;
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+/* FIXME: We need to also provide an encode/decode X-Evolution function, as the default
+   is no good for us, and can screw up the uid info */
+
 static CamelMessageInfo *message_info_new(CamelFolderSummary * s, struct _header_raw *h)
 {
 	CamelMessageInfo *mi;
@@ -144,13 +209,41 @@ static CamelMessageInfo *message_info_new(CamelFolderSummary * s, struct _header
 	if (mi) {
 		mdi = (CamelMaildirMessageInfo *)mi;
 
-		mi->uid = camel_folder_summary_next_uid_string(s);
+		if (mi->uid == NULL) {
+			mi->uid = camel_folder_summary_next_uid_string(s);
+		}
+
+		/* with maildir we know the real received date, from the filename */
+		mi->date_received = strtoul(mi->uid, NULL, 10);
 
-		/* should store some status info in the filename, but we wont (yet) (fixme) */
 		if (mds->priv->current_file) {
+#if 0
+			char *p1, *p2, *p3;
+			unsigned long uid;
+#endif
+			/* if setting from a file, grab the flags from it */
 			mdi->filename = g_strdup(mds->priv->current_file);
+			camel_maildir_summary_name_to_info(mi, mdi->filename);
+
+#if 0
+			/* Actually, I dont think all this effort is worth it at all ... */
+
+			/* also, see if we can extract the next-id from tne name, and safe-if-fy ourselves against collisions */
+			/* we check for something.something_number.something */
+			p1 = strchr(mdi->filename, '.');
+			if (p1) {
+				p2 = strchr(p1+1, '.');
+				p3 = strchr(p1+1, '_');
+				if (p2 && p3 && p3<p2) {
+					uid = strtoul(p3+1, &p1, 10);
+					if (p1 == p2 && uid>0)
+						camel_folder_summary_set_uid(s, uid);
+				}
+			}
+#endif
 		} else {
-			mdi->filename = g_strdup_printf("%s:2,", mi->uid);
+			/* if creating a file, set its name from the flags we have */
+			mdi->filename = camel_maildir_summary_info_to_name(mi);
 		}
 	}
 
@@ -160,9 +253,10 @@ static CamelMessageInfo *message_info_new(CamelFolderSummary * s, struct _header
 static char *maildir_summary_next_uid_string(CamelFolderSummary *s)
 {
 	CamelMaildirSummary *mds = (CamelMaildirSummary *)s;
-	/*CamelLocalSummary *cls = (CamelLocalSummary *)s;*/
 
-	/* current_file is more a current_filename, so map the filename to a uid */
+	d(printf("next uid string called?\n"));
+
+	/* if we have a current file, then use that to get the uid */
 	if (mds->priv->current_file) {
 		char *cln;
 
@@ -172,8 +266,33 @@ static char *maildir_summary_next_uid_string(CamelFolderSummary *s)
 		else
 			return g_strdup(mds->priv->current_file);
 	} else {
-		/* we use time.pid_count.hostname */
+		/* the first would probably work, but just to be safe, check for collisions */
+#if 0
 		return g_strdup_printf("%ld.%d_%u.%s", time(0), getpid(), camel_folder_summary_next_uid(s), mds->priv->hostname);
+#else
+		CamelLocalSummary *cls = (CamelLocalSummary *)s;
+		char *name = NULL, *uid = NULL;
+		struct stat st;
+		int retry = 0;
+		guint32 nextuid = camel_folder_summary_next_uid(s);
+
+		/* we use time.pid_count.hostname */
+		do {
+			if (retry > 0) {
+				g_free(name);
+				g_free(uid);
+				sleep(2);
+			}
+			uid = g_strdup_printf("%ld.%d_%u.%s", time(0), getpid(), nextuid, mds->priv->hostname);
+			name = g_strdup_printf("%s/tmp/%s", cls->folder_path, uid);
+			retry++;
+		} while (stat(name, &st) == 0 && retry<3);
+
+		/* I dont know what we're supposed to do if it fails to find a unique name?? */
+
+		g_free(name);
+		return uid;
+#endif
 	}
 }
 
@@ -220,6 +339,21 @@ remove_summary(char *key, CamelMessageInfo *info, CamelLocalSummary *cls)
 }
 
 static int
+sort_receive_cmp(const void *ap, const void *bp)
+{
+	const CamelMessageInfo
+		*a = *((CamelMessageInfo **)ap),
+		*b = *((CamelMessageInfo **)bp);
+
+	if (a->date_received < b->date_received)
+		return -1;
+	else if (a->date_received > b->date_received)
+		return 1;
+
+	return 0;
+}
+
+static int
 maildir_summary_check(CamelLocalSummary *cls, CamelFolderChangeInfo *changes, CamelException *ex)
 {
 	DIR *dir;
@@ -343,6 +477,9 @@ maildir_summary_check(CamelLocalSummary *cls, CamelFolderChangeInfo *changes, Ca
 	g_free(new);
 	g_free(cur);
 
+	/* sort the summary based on receive time, since the directory order is not useful */
+	qsort(s->messages->pdata, s->messages->len, sizeof(CamelMessageInfo *), sort_receive_cmp);
+
 	/* FIXME: move this up a class? */
 
 	/* force a save of the index, just to make sure */
@@ -355,8 +492,7 @@ maildir_summary_check(CamelLocalSummary *cls, CamelFolderChangeInfo *changes, Ca
 	return 0;
 }
 
-/* sync the summary with the ondisk files.
-   It doesnt store the state in the file, the summary only, == MUCH faster */
+/* sync the summary with the ondisk files. */
 static int
 maildir_summary_sync(CamelLocalSummary *cls, gboolean expunge, CamelFolderChangeInfo *changes, CamelException *ex)
 {
@@ -364,20 +500,19 @@ maildir_summary_sync(CamelLocalSummary *cls, gboolean expunge, CamelFolderChange
 	CamelMessageInfo *info;
 	CamelMaildirMessageInfo *mdi;
 	char *name;
+	struct stat st;
 
 	d(printf("summary_sync(expunge=%s)\n", expunge?"true":"false"));
 
 	if (cls->index) {
 		ibex_save(cls->index);
 	}
-	if (!expunge)
-		return 0;
 
 	count = camel_folder_summary_count((CamelFolderSummary *)cls);
 	for (i=count-1;i>=0;i--) {
 		info = camel_folder_summary_index((CamelFolderSummary *)cls, i);
-		if (info && info->flags & CAMEL_MESSAGE_DELETED) {
-			mdi = (CamelMaildirMessageInfo *)info;
+		mdi = (CamelMaildirMessageInfo *)info;
+		if (info && (info->flags & CAMEL_MESSAGE_DELETED) && expunge) {
 			name = g_strdup_printf("%s/cur/%s", cls->folder_path, mdi->filename);
 			d(printf("deleting %s\n", name));
 			if (unlink(name) == 0 || errno==ENOENT) {
@@ -389,6 +524,34 @@ maildir_summary_sync(CamelLocalSummary *cls, gboolean expunge, CamelFolderChange
 				camel_folder_change_info_remove_uid(changes, info->uid);
 				camel_folder_summary_remove((CamelFolderSummary *)cls, info);
 			}
+			g_free(name);
+		} else if (info && (info->flags & CAMEL_MESSAGE_FOLDER_FLAGGED)) {
+			char *newname = camel_maildir_summary_info_to_name(info);
+			char *dest;
+
+			/* do we care about additional metainfo stored inside the message? */
+			/* probably should all go in the filename? */
+
+			/* have our flags/ i.e. name changed? */
+			if (strcmp(newname, mdi->filename)) {
+				name = g_strdup_printf("%s/cur/%s", cls->folder_path, mdi->filename);
+				dest = g_strdup_printf("%s/cur/%s", cls->folder_path, newname);
+				rename(name, dest);
+				if (stat(dest, &st) == -1) {
+					/* we'll assume it didn't work, but dont change anything else */
+					g_free(newname);
+				} else {
+					g_free(mdi->filename);
+					mdi->filename = newname;
+				}
+				g_free(name);
+				g_free(dest);
+			} else {
+				g_free(newname);
+			}
+
+			/* strip FOLDER_MESSAGE_FLAGED, etc */
+			info->flags &= 0xffff;
 		}
 	}
 	return 0;
diff --git a/camel/providers/local/camel-maildir-summary.h b/camel/providers/local/camel-maildir-summary.h
index e1f00ac385..25ea845c21 100644
--- a/camel/providers/local/camel-maildir-summary.h
+++ b/camel/providers/local/camel-maildir-summary.h
@@ -59,5 +59,9 @@ struct _CamelMaildirSummaryClass {
 CamelType	 camel_maildir_summary_get_type	(void);
 CamelMaildirSummary	*camel_maildir_summary_new	(const char *filename, const char *maildirdir, ibex *index);
 
+/* convert some info->flags to/from the messageinfo */
+char *camel_maildir_summary_info_to_name(const CamelMessageInfo *info);
+int camel_maildir_summary_name_to_info(CamelMessageInfo *info, const char *name);
+
 #endif /* ! _CAMEL_MAILDIR_SUMMARY_H */
 
diff --git a/camel/providers/local/camel-mbox-folder.c b/camel/providers/local/camel-mbox-folder.c
index d12b6431de..d65ad54c01 100644
--- a/camel/providers/local/camel-mbox-folder.c
+++ b/camel/providers/local/camel-mbox-folder.c
@@ -51,6 +51,8 @@ static CamelLocalFolderClass *parent_class = NULL;
 #define CF_CLASS(so) CAMEL_FOLDER_CLASS (CAMEL_OBJECT_GET_CLASS(so))
 #define CMBOXS_CLASS(so) CAMEL_STORE_CLASS (CAMEL_OBJECT_GET_CLASS(so))
 
+static int mbox_lock(CamelLocalFolder *lf, CamelLockType type, CamelException *ex);
+static void mbox_unlock(CamelLocalFolder *lf);
 
 static void mbox_append_message(CamelFolder *folder, CamelMimeMessage * message, const CamelMessageInfo * info,	CamelException *ex);
 static CamelMimeMessage *mbox_get_message(CamelFolder *folder, const gchar * uid, CamelException *ex);
@@ -73,19 +75,25 @@ camel_mbox_folder_class_init(CamelMboxFolderClass * camel_mbox_folder_class)
 	camel_folder_class->get_message = mbox_get_message;
 
 	lclass->create_summary = mbox_create_summary;
+	lclass->lock = mbox_lock;
+	lclass->unlock = mbox_unlock;
 }
 
 static void
 mbox_init(gpointer object, gpointer klass)
 {
-	/*CamelFolder *folder = object;
-	  CamelMboxFolder *mbox_folder = object;*/
+	/*CamelFolder *folder = object;*/
+	CamelMboxFolder *mbox_folder = object;
+
+	mbox_folder->lockfd = -1;
 }
 
 static void
 mbox_finalise(CamelObject * object)
 {
-	/*CamelMboxFolder *mbox_folder = CAMEL_MBOX_FOLDER(object);*/
+	CamelMboxFolder *mbox_folder = (CamelMboxFolder *)object;
+
+	g_assert(mbox_folder->lockfd == -1);
 }
 
 CamelType camel_mbox_folder_get_type(void)
@@ -124,6 +132,32 @@ static CamelLocalSummary *mbox_create_summary(const char *path, const char *fold
 	return (CamelLocalSummary *)camel_mbox_summary_new(path, folder, index);
 }
 
+static int mbox_lock(CamelLocalFolder *lf, CamelLockType type, CamelException *ex)
+{
+	CamelMboxFolder *mf = (CamelMboxFolder *)lf;
+
+	/* make sure we have matching unlocks for locks, camel-local-folder class should enforce this */
+	g_assert(mf->lockfd == -1);
+
+	mf->lockfd = open(lf->folder_path, O_RDWR, 0);
+	if (mf->lockfd == -1) {
+		camel_exception_setv(ex, 1, "Cannot create folder lock on %s: %s", lf->folder_path, strerror(errno));
+		return -1;
+	}
+
+	return camel_lock_folder(lf->folder_path, mf->lockfd, type, ex);
+}
+
+static void mbox_unlock(CamelLocalFolder *lf)
+{
+	CamelMboxFolder *mf = (CamelMboxFolder *)lf;
+
+	g_assert(mf->lockfd != -1);
+	camel_unlock_folder(lf->folder_path, mf->lockfd);
+	close(mf->lockfd);
+	mf->lockfd = -1;
+}
+
 static void
 mbox_append_message(CamelFolder *folder, CamelMimeMessage * message, const CamelMessageInfo * info, CamelException *ex)
 {
@@ -136,27 +170,28 @@ mbox_append_message(CamelFolder *folder, CamelMimeMessage * message, const Camel
 	int fd;
 	struct stat st;
 
-	/* FIXME: Locking */
+	/* If we can't lock, dont do anything */
+	if (camel_local_folder_lock(lf, CAMEL_LOCK_WRITE, ex) == -1)
+		return;
 
 	d(printf("Appending message\n"));
 
 	/* first, check the summary is correct (updates folder_size too) */
 	camel_local_summary_check(lf->summary, lf->changes, ex);
 	if (camel_exception_is_set(ex))
-		return;
+		goto fail;
 
 	/* add it to the summary/assign the uid, etc */
 	mi = camel_local_summary_add(lf->summary, message, info, lf->changes, ex);
-	if (camel_exception_is_set(ex)) {
-		return;
-	}
+	if (camel_exception_is_set(ex))
+		goto fail;
 
 	d(printf("Appending message: uid is %s\n", mi->uid));
 
 	output_stream = camel_stream_fs_new_with_name(lf->folder_path, O_WRONLY|O_APPEND, 0600);
 	if (output_stream == NULL) {
 		camel_exception_setv(ex, 1, _("Cannot open mailbox: %s: %s\n"), lf->folder_path, strerror(errno));
-		return;
+		goto fail;
 	}
 
 	/* we must write this to the non-filtered stream ... prepend a \n if not at the start of the file */
@@ -174,12 +209,8 @@ mbox_append_message(CamelFolder *folder, CamelMimeMessage * message, const Camel
 	if (camel_stream_close(filter_stream) == -1)
 		goto fail_write;
 
-	/* now we 'fudge' the summary  to tell it its uptodate, because its idea of uptodate has just changed */
-	/* the stat really shouldn't fail, we just wrote to it */
-	if (stat(lf->folder_path, &st) == 0) {
-		mbs->folder_size = st.st_size;
-		((CamelFolderSummary *)mbs)->time = st.st_mtime;
-	}
+	/* unlock as soon as we can */
+	camel_local_folder_unlock(lf);
 
 	/* filter stream ref's the output stream itself, so we need to unref it too */
 	camel_object_unref((CamelObject *)filter_from);
@@ -187,6 +218,13 @@ mbox_append_message(CamelFolder *folder, CamelMimeMessage * message, const Camel
 	camel_object_unref((CamelObject *)output_stream);
 	g_free(fromline);
 
+	/* now we 'fudge' the summary  to tell it its uptodate, because its idea of uptodate has just changed */
+	/* the stat really shouldn't fail, we just wrote to it */
+	if (stat(lf->folder_path, &st) == 0) {
+		mbs->folder_size = st.st_size;
+		((CamelFolderSummary *)mbs)->time = st.st_mtime;
+	}
+
 	if (camel_folder_change_info_changed(lf->changes)) {
 		camel_object_trigger_event((CamelObject *)folder, "folder_changed", lf->changes);
 		camel_folder_change_info_clear(lf->changes);
@@ -223,6 +261,10 @@ fail_write:
 		((CamelFolderSummary *)mbs)->time = st.st_mtime;
 	}
 
+fail:
+	/* make sure we unlock the folder - before we start triggering events into appland */
+	camel_local_folder_unlock(lf);
+
 	/* cascade the changes through, anyway, if there are any outstanding */
 	if (camel_folder_change_info_changed(lf->changes)) {
 		camel_object_trigger_event((CamelObject *)folder, "folder_changed", lf->changes);
@@ -242,7 +284,9 @@ mbox_get_message(CamelFolder *folder, const gchar * uid, CamelException *ex)
 	
 	d(printf("Getting message %s\n", uid));
 
-	/* FIXME: mbox locking */
+	/* lock the folder first, burn if we can't */
+	if (camel_local_folder_lock(lf, CAMEL_LOCK_READ, ex) == -1)
+		return NULL;
 	
 retry:
 	/* get the message summary info */
@@ -251,6 +295,7 @@ retry:
 	if (info == NULL) {
 		camel_exception_setv(ex, CAMEL_EXCEPTION_FOLDER_INVALID_UID,
 				     _("Cannot get message: %s\n  %s"), uid, _("No such message"));
+		camel_local_folder_unlock(lf);
 		return NULL;
 	}
 
@@ -260,13 +305,15 @@ retry:
 	
 	/* we use an fd instead of a normal stream here - the reason is subtle, camel_mime_part will cache
 	   the whole message in memory if the stream is non-seekable (which it is when built from a parser
-	   with no stream).  This means we dont have to lock the mbox for the life of the message. */
+	   with no stream).  This means we dont have to lock the mbox for the life of the message, but only
+	   while it is being created. */
 
 	fd = open(lf->folder_path, O_RDONLY);
 	if (fd == -1) {
 		camel_exception_setv(ex, CAMEL_EXCEPTION_FOLDER_INVALID_UID,
 				     _("Cannot get message: %s from folder %s\n  %s"), uid, lf->folder_path,
 				     strerror(errno));
+		camel_local_folder_unlock(lf);
 		return NULL;
 	}
 
@@ -296,6 +343,7 @@ retry:
 				     _("Cannot get message: %s from folder %s\n  %s"), uid, lf->folder_path,
 				     _("The folder appears to be irrecoverably corrupted."));
 
+		camel_local_folder_unlock(lf);
 		return NULL;
 	}
 	
@@ -307,9 +355,13 @@ retry:
 				     _("Message construction failed: Corrupt mailbox?"));
 		camel_object_unref((CamelObject *)parser);
 		camel_object_unref((CamelObject *)message);
+		camel_local_folder_unlock(lf);
 		return NULL;
 	}
 
+	/* and unlock now we're finished with it */
+	camel_local_folder_unlock(lf);
+
 	camel_object_unref((CamelObject *)parser);
 	
 	/* use the opportunity to notify of changes (particularly if we had a rebuild) */
diff --git a/camel/providers/local/camel-mbox-folder.h b/camel/providers/local/camel-mbox-folder.h
index 573b5df177..3ef31a843c 100644
--- a/camel/providers/local/camel-mbox-folder.h
+++ b/camel/providers/local/camel-mbox-folder.h
@@ -39,6 +39,7 @@ extern "C" {
 typedef struct {
 	CamelLocalFolder parent_object;
 
+	int lockfd;		/* for when we have a lock on the folder */
 } CamelMboxFolder;
 
 typedef struct {
diff --git a/camel/providers/local/camel-mh-folder.c b/camel/providers/local/camel-mh-folder.c
index c3b74dc3d6..8d6ce69c3c 100644
--- a/camel/providers/local/camel-mh-folder.c
+++ b/camel/providers/local/camel-mh-folder.c
@@ -127,7 +127,7 @@ static void mh_append_message(CamelFolder * folder, CamelMimeMessage * message,
 	CamelMessageInfo *mi;
 	char *name;
 
-	/* FIXME: probably needs additional locking */
+	/* FIXME: probably needs additional locking (although mh doesn't appear do do it) */
 
 	d(printf("Appending message\n"));