Sync the key tables explcitly.

2002-04-03  Not Zed  <NotZed@Ximian.com>

        * camel-text-index.c (text_index_sync): Sync the key tables
        explcitly.

        * camel-partition-table.c (camel_key_table_finalise): Sync root
        block when done.
        (camel_key_table_sync): New function, sync key table (root) explicitly.

2002-04-02  Not Zed  <NotZed@Ximian.com>

        * camel-block-file.c (camel_block_file_free_block): Mark root
        block dirty when we change it (this function isn't used yet
        anyway).

        * camel-text-index.c (text_index_add_name_to_word): Touch the root
        block when we modify the counts.  Also, abort processing on any
        errors.
        (text_index_sync): Fix typo in comments.  Sync the block file
        inside the lock.
        (text_index_compress_nosync): Lock the old index while we're
        compressing.
        (text_index_compress_nosync): Remove the bogus while() at the end
        of the while() loops!  Also plug a memleak - records weren't
        freed.
        (text_index_rename): Lock around rename op.
        (text_index_add_name): More typos.
        (text_index_sync): Touch root when changing it.
        (text_index_add_name): "
        (text_index_delete_name): "
        (camel_text_index_new): Touch root if we change it.
        (text_index_cursor_reset): Make sure we NULL pointers after we
        free them (nothing uses this yet).

        * camel-partition-table.c (hash_key): Remove some debug
        accidentally left in.
        (camel_partition_table_add): When linking in the next block list,
        set the right previous pointer.
        (camel_key_table_add): Simplify the 'left' calculation (it was
        already ok though).
        (camel_key_table_next): Initialise returns before processing.
        Broaden the lock slightly, and simplify validity calculations.

svn path=/trunk/; revision=16323

5 files changed, 147 insertions, 34 deletions

diff --git a/camel/ChangeLog b/camel/ChangeLog
index 5c19d9be72..8c1a6cdc07 100644
--- a/camel/ChangeLog
+++ b/camel/ChangeLog
@@ -1,5 +1,46 @@
+2002-04-03  Not Zed  <NotZed@Ximian.com>
+
+	* camel-text-index.c (text_index_sync): Sync the key tables
+	explcitly.
+
+	* camel-partition-table.c (camel_key_table_finalise): Sync root
+	block when done.
+	(camel_key_table_sync): New function, sync key table (root) explicitly.
+
 2002-04-02  Not Zed  <NotZed@Ximian.com>
 
+	* camel-block-file.c (camel_block_file_free_block): Mark root
+	block dirty when we change it (this function isn't used yet
+	anyway).
+
+	* camel-text-index.c (text_index_add_name_to_word): Touch the root
+	block when we modify the counts.  Also, abort processing on any
+	errors.
+	(text_index_sync): Fix typo in comments.  Sync the block file
+	inside the lock.
+	(text_index_compress_nosync): Lock the old index while we're
+	compressing.
+	(text_index_compress_nosync): Remove the bogus while() at the end
+	of the while() loops!  Also plug a memleak - records weren't
+	freed.
+	(text_index_rename): Lock around rename op.
+	(text_index_add_name): More typos.
+	(text_index_sync): Touch root when changing it.
+	(text_index_add_name): "
+	(text_index_delete_name): "
+	(camel_text_index_new): Touch root if we change it.
+	(text_index_cursor_reset): Make sure we NULL pointers after we
+	free them (nothing uses this yet).
+
+	* camel-partition-table.c (hash_key): Remove some debug
+	accidentally left in.
+	(camel_partition_table_add): When linking in the next block list,
+	set the right previous pointer.
+	(camel_key_table_add): Simplify the 'left' calculation (it was
+	already ok though).
+	(camel_key_table_next): Initialise returns before processing.
+	Broaden the lock slightly, and simplify validity calculations.
+
 	* providers/imap/camel-imap-store.c (imap_keepalive): Put back in
 	the exception setup stuff i disabled for debugging.
 
diff --git a/camel/camel-block-file.c b/camel/camel-block-file.c
index 0340f9ecdf..e73b266026 100644
--- a/camel/camel-block-file.c
+++ b/camel/camel-block-file.c
@@ -98,11 +98,22 @@ block_file_validate_root(CamelBlockFile *bs)
 
 	fstat(bs->fd, &st);
 
-	d(printf("Validate root:\n"));
-	d(printf("version: %.8s (%.8s)\n", bs->root->version, bs->version));
-	d(printf("block size: %d (%d)\n", br->block_size, bs->block_size));
-	d(printf("free: %d (%d add size < %d)\n", br->free, br->free / bs->block_size * bs->block_size, (int)st.st_size));
-	d(printf("last: %d (%d and size: %d)\n", br->free, br->free / bs->block_size * bs->block_size, (int)st.st_size));
+	(printf("Validate root:\n"));
+	(printf("version: %.8s (%.8s)\n", bs->root->version, bs->version));
+	(printf("block size: %d (%d)%s\n", br->block_size, bs->block_size,
+		br->block_size != bs->block_size ? " BAD":" OK"));
+	(printf("free: %ld (%d add size < %ld)%s\n", (long)br->free, br->free / bs->block_size * bs->block_size, (long)st.st_size,
+		(br->free > st.st_size) || (br->free % bs->block_size) != 0 ? " BAD":" OK"));
+	(printf("last: %ld (%d and size: %ld)%s\n", (long)br->last, br->last / bs->block_size * bs->block_size, (long)st.st_size,
+		(br->last != st.st_size) || ((br->last % bs->block_size) != 0) ? " BAD": " OK"));
+	(printf("flags: %s\n", (br->flags & CAMEL_BLOCK_FILE_SYNC)?"SYNC":"unSYNC"));
+
+	printf("last = %ld, size = %ld\n", (unsigned long)br->last, (unsigned long)st.st_size);
+
+	if (br->last != st.st_size)
+		printf("last != size!\n");
+	else
+		printf("last == size?\n");
 
 	if (br->last == 0
 	    || memcmp(bs->root->version, bs->version, 8) != 0
@@ -476,6 +487,7 @@ int camel_block_file_free_block(CamelBlockFile *bs, camel_block_t id)
 
 	((camel_block_t *)bl->data)[0] = bs->root->free;
 	bs->root->free = bl->id;
+	bs->root_block->flags |= CAMEL_BLOCK_DIRTY;
 	bl->flags |= CAMEL_BLOCK_DIRTY;
 	camel_block_file_unref_block(bs, bl);
 
diff --git a/camel/camel-partition-table.c b/camel/camel-partition-table.c
index c386cbcafa..eb44d9c197 100644
--- a/camel/camel-partition-table.c
+++ b/camel/camel-partition-table.c
@@ -40,7 +40,7 @@
 
 /* Do we synchronously write table updates - makes the
    tables consistent after program crash without sync */
-#define SYNC_UPDATES
+/*#define SYNC_UPDATES*/
 
 #ifdef ENABLE_THREADS
 #include <pthread.h>
@@ -141,19 +141,11 @@ lookup word, if nameid is deleted, mark it in wordlist as unused and mark for wr
 
 /* ********************************************************************** */
 
-void
-camel_break_here(void)
-{
-}
-
 /* This simple hash seems to work quite well */
 static camel_hash_t hash_key(const char *key)
 {
 	camel_hash_t hash = 0xABADF00D;
 
-	if (strcmp(key, "4852") == 0)
-		camel_break_here();
-
 	while (*key) {
 		hash = hash * (*key) ^ (*key);
 		key++;
@@ -477,7 +469,7 @@ camel_partition_table_add(CamelPartitionTable *cpi, const char *key, camel_key_t
 
 				/* link in-memory */
 				ptnblock->next = ptblock->next;
-				ptblock->next->prev = ptblock;
+				ptblock->next->prev = ptnblock;
 				ptblock->next = ptnblock;
 				ptnblock->prev = ptblock;
 
@@ -622,8 +614,10 @@ camel_key_table_finalise(CamelKeyTable *ki)
 	p = ki->priv;
 
 	if (ki->blocks) {
-		if (ki->root_block)
+		if (ki->root_block) {
+			camel_block_file_sync_block(ki->blocks, ki->root_block);
 			camel_block_file_unref_block(ki->blocks, ki->root_block);
+		}
 		camel_block_file_sync(ki->blocks);
 		camel_object_unref((CamelObject *)ki->blocks);
 	}
@@ -680,6 +674,16 @@ camel_key_table_new(CamelBlockFile *bs, camel_block_t root)
 	return ki;
 }
 
+int
+camel_key_table_sync(CamelKeyTable *ki)
+{
+#ifdef SYNC_UPDATES
+	return 0;
+#else
+	return camel_block_file_sync_block(ki->blocks, ki->root_block);
+#endif
+}
+
 camel_key_t
 camel_key_table_add(CamelKeyTable *ki, const char *key, camel_block_t data, unsigned int flags)
 {
@@ -715,7 +719,8 @@ camel_key_table_add(CamelKeyTable *ki, const char *key, camel_block_t data, unsi
 		goto fail;
 
 	if (kblast->used > 0) {
-		left = &kblast->u.keydata[kblast->u.keys[kblast->used-1].offset] - (char *)(&kblast->u.keys[kblast->used+1]);
+		/*left = &kblast->u.keydata[kblast->u.keys[kblast->used-1].offset] - (char *)(&kblast->u.keys[kblast->used+1]);*/
+		left = kblast->u.keys[kblast->used-1].offset - sizeof(kblast->u.keys[0])*(kblast->used+1);
 		d(printf("used = %d (%d), filled = %d, left = %d  len = %d?\n",
 			 kblast->used, kblast->used * sizeof(kblast->u.keys[0]),
 			 sizeof(kblast->u.keydata) - kblast->u.keys[kblast->used-1].offset,
@@ -901,6 +906,15 @@ camel_key_table_next(CamelKeyTable *ki, camel_key_t next, char **keyp, unsigned
 	camel_block_t blockid;
 	int index;
 
+	if (keyp)
+		*keyp = 0;
+	if (flagsp)
+		*flagsp = 0;
+	if (datap)
+		*datap = 0;
+
+	CAMEL_KEY_TABLE_LOCK(ki, lock);
+
 	if (next == 0) {
 		next = ki->root->first;
 		if (next == 0)
@@ -913,8 +927,10 @@ camel_key_table_next(CamelKeyTable *ki, camel_key_t next, char **keyp, unsigned
 		index = next & (CAMEL_BLOCK_SIZE-1);
 		
 		bl = camel_block_file_get_block(ki->blocks, blockid);
-		if (bl == NULL)
+		if (bl == NULL) {
+			CAMEL_KEY_TABLE_UNLOCK(ki, lock);
 			return 0;
+		}
 
 		kb = (CamelKeyBlock *)&bl->data;
 
@@ -927,14 +943,14 @@ camel_key_table_next(CamelKeyTable *ki, camel_key_t next, char **keyp, unsigned
 		}
 	} while (bl == NULL);
 
-	CAMEL_KEY_TABLE_LOCK(ki, lock);
-
 	/* invalid block data */
 	if ((kb->u.keys[index].offset >= sizeof(kb->u.keydata)
-	     || kb->u.keys[index].offset < kb->u.keydata - (char *)&kb->u.keys[kb->used])
+	     /*|| kb->u.keys[index].offset < kb->u.keydata - (char *)&kb->u.keys[kb->used])*/
+	     || kb->u.keys[index].offset < sizeof(kb->u.keys[0]) * kb->used
 	    || (index > 0 &&
 		(kb->u.keys[index-1].offset >= sizeof(kb->u.keydata)
-		 || kb->u.keys[index-1].offset < kb->u.keydata - (char *)&kb->u.keys[kb->used]))) {
+		 /*|| kb->u.keys[index-1].offset < kb->u.keydata - (char *)&kb->u.keys[kb->used]))) {*/
+		 || kb->u.keys[index-1].offset < sizeof(kb->u.keys[0]) * kb->used)))) {
 		g_warning("Block %u invalid scanning keys", bl->id);
 		camel_block_file_unref_block(ki->blocks, bl);
 		CAMEL_KEY_TABLE_UNLOCK(ki, lock);
diff --git a/camel/camel-partition-table.h b/camel/camel-partition-table.h
index ab6ac8c204..04e2b6cd88 100644
--- a/camel/camel-partition-table.h
+++ b/camel/camel-partition-table.h
@@ -141,6 +141,7 @@ struct _CamelKeyTableClass {
 CamelType camel_key_table_get_type(void);
 
 CamelKeyTable * camel_key_table_new(CamelBlockFile *bs, camel_block_t root);
+int camel_key_table_sync(CamelKeyTable *ki);
 camel_key_t camel_key_table_add(CamelKeyTable *ki, const char *key, camel_block_t data, unsigned int flags);
 void camel_key_table_set_data(CamelKeyTable *ki, camel_key_t keyid, camel_block_t data);
 void camel_key_table_set_flags(CamelKeyTable *ki, camel_key_t keyid, unsigned int flags, unsigned int set);
diff --git a/camel/camel-text-index.c b/camel/camel-text-index.c
index 5b5a349c84..dadc4de083 100644
--- a/camel/camel-text-index.c
+++ b/camel/camel-text-index.c
@@ -193,14 +193,22 @@ text_index_add_name_to_word(CamelIndex *idx, const char *word, camel_key_t namei
 		if (wordid == 0) {
 			data = 0;
 			wordid = camel_key_table_add(p->word_index, word, 0, 0);
-			if (wordid == 0)
+			if (wordid == 0){
+				g_warning("Could not create key entry for word '%s': %s\n", word, strerror(errno));
 				return;
-			camel_partition_table_add(p->word_hash, word, wordid);
+			}
+			if (camel_partition_table_add(p->word_hash, word, wordid) == -1) {
+				g_warning("Could not create hash entry for word '%s': %s\n", word, strerror(errno));
+				return;
+			}
 			rb->words++;
+			camel_block_file_touch_block(p->blocks, p->blocks->root_block);
 		} else {
 			data = camel_key_table_lookup(p->word_index, wordid, NULL, 0);
-			if (data == 0)
+			if (data == 0) {
+				g_warning("Could not find key entry for word '%s': %s\n", word, strerror(errno));
 				return;
+			}
 		}
 		
 		w = g_malloc0(sizeof(*w));
@@ -220,6 +228,8 @@ text_index_add_name_to_word(CamelIndex *idx, const char *word, camel_key_t namei
 			if (camel_key_file_write(p->links, &ww->data, ww->used, ww->names) != -1) {
 				io(printf("  new data [%x]\n", ww->data));
 				rb->keys++;
+				camel_block_file_touch_block(p->blocks, p->blocks->root_block);
+				/* if this call fails - we still point to the old data - not fatal */
 				camel_key_table_set_data(p->word_index, ww->wordid, ww->data);
 				e_dlist_remove((EDListNode *)ww);
 				g_hash_table_remove(p->words, ww->word);
@@ -239,6 +249,8 @@ text_index_add_name_to_word(CamelIndex *idx, const char *word, camel_key_t namei
 			io(printf("writing key file entry '%s' [%x]\n", w->word, w->data));
 			if (camel_key_file_write(p->links, &w->data, w->used, w->names) != -1) {
 				rb->keys++;
+				camel_block_file_touch_block(p->blocks, p->blocks->root_block);
+				/* if this call fails - we still point to the old data - not fatal */
 				camel_key_table_set_data(p->word_index, w->wordid, w->data);
 			}
 			/* FIXME: what to on error?  lost data? */
@@ -264,7 +276,7 @@ text_index_sync(CamelIndex *idx)
 
 	CAMEL_TEXT_INDEX_LOCK(idx, lock);
 
-	/* we sync, bump down the cahce limits since we dont need them for reading */
+	/* we sync, bump down the cache limits since we dont need them for reading */
 	p->blocks->block_cache_limit = 128;
 	/* this doesn't really need to be dropped, its only used in updates anyway */
 	p->word_cache_limit = 1024;
@@ -277,6 +289,7 @@ text_index_sync(CamelIndex *idx)
 			if (camel_key_file_write(p->links, &ww->data, ww->used, ww->names) != -1) {
 				io(printf("  new data [%x]\n", ww->data));
 				rb->keys++;
+				camel_block_file_touch_block(p->blocks, p->blocks->root_block);
 				camel_key_table_set_data(p->word_index, ww->wordid, ww->data);
 			} else {
 				ret = -1;
@@ -288,6 +301,10 @@ text_index_sync(CamelIndex *idx)
 		g_free(ww);
 	}	
 
+	if (camel_key_table_sync(p->word_index) == -1
+	    || camel_key_table_sync(p->name_index) == -1)
+		ret = -1;
+
 	/* only do the frag/compress check if we did some new writes on this index */
 	if (ret == 0 && work) {
 		wfrag = rb->words ? (((rb->keys - rb->words) * 100)/ rb->words) : 0;
@@ -298,9 +315,11 @@ text_index_sync(CamelIndex *idx)
 			ret = text_index_compress_nosync(idx);
 	}
 
+	ret = camel_block_file_sync(p->blocks);
+
 	CAMEL_TEXT_INDEX_UNLOCK(idx, lock);
 
-	return camel_block_file_sync(p->blocks);
+	return ret;
 }
 
 static void tmp_name(const char *in, char *o)
@@ -365,6 +384,8 @@ text_index_compress_nosync(CamelIndex *idx)
 	newp = CTI_PRIVATE(newidx);
 	oldp = CTI_PRIVATE(idx);
 
+	CAMEL_TEXT_INDEX_LOCK(idx, lock);
+
 	rb = (struct _CamelTextIndexRoot *)newp->blocks->root;
 
 	rb->words = 0;
@@ -399,7 +420,7 @@ text_index_compress_nosync(CamelIndex *idx)
 		g_free(name);
 		name = NULL;
 		deleted |= flags;
-	} while (oldkeyid);
+	}
 
 	/* Copy word data across, remapping/deleting and create new index for it */
 	/* We re-block the data into 256 entry lots while we're at it, since we only
@@ -423,12 +444,15 @@ text_index_compress_nosync(CamelIndex *idx)
 				if (newkeyid) {
 					newrecords[newcount++] = newkeyid;
 					if (newcount == sizeof(newrecords)/sizeof(newrecords[0])) {
-						if (camel_key_file_write(newp->links, &newdata, newcount, newrecords) == -1)
+						if (camel_key_file_write(newp->links, &newdata, newcount, newrecords) == -1) {
+							g_free(records);
 							goto fail;
+						}
 						newcount = 0;
 					}
 				}
 			}
+			g_free(records);
 		}
 		
 		if (newcount > 0) {
@@ -442,7 +466,9 @@ text_index_compress_nosync(CamelIndex *idx)
 		camel_partition_table_add(newp->word_hash, name, newkeyid);
 		g_free(name);
 		name = NULL;
-	} while (oldkeyid);
+	}
+
+	camel_block_file_touch_block(newp->blocks, newp->blocks->root_block);
 	
 	if (camel_index_sync((CamelIndex *)newidx) == -1)
 		goto fail;
@@ -468,6 +494,8 @@ text_index_compress_nosync(CamelIndex *idx)
 
 	ret = 0;
 fail:
+	CAMEL_TEXT_INDEX_UNLOCK(idx, lock);
+
 	camel_object_unref((CamelObject *)newidx);
 	g_free(name);
 	g_hash_table_destroy(remap);
@@ -489,11 +517,13 @@ text_index_rename(CamelIndex *idx, const char *path)
 	char *newlink;
 	int err, ret;
 
-	/* TODO: Needs some lock? */
+	CAMEL_TEXT_INDEX_LOCK(idx, lock);
 
 	ret = camel_block_file_rename(p->blocks, path);
-	if (ret == -1)
+	if (ret == -1) {
+		CAMEL_TEXT_INDEX_UNLOCK(idx, lock);
 		return -1;
+	}
 
 	newlink = alloca(strlen(path)+8);
 	sprintf(newlink, "%s.data", path);
@@ -501,6 +531,7 @@ text_index_rename(CamelIndex *idx, const char *path)
 	if (ret == -1) {
 		err = errno;
 		camel_block_file_rename(p->blocks, path);
+		CAMEL_TEXT_INDEX_UNLOCK(idx, lock);
 		errno = err;
 		return -1;
 	}
@@ -508,6 +539,8 @@ text_index_rename(CamelIndex *idx, const char *path)
 	g_free(idx->path);
 	idx->path = g_strdup(path);
 
+	CAMEL_TEXT_INDEX_UNLOCK(idx, lock);
+
 	return 0;
 }
 
@@ -538,6 +571,8 @@ text_index_add_name(CamelIndex *idx, const char *name)
 	/* If we have it already replace it */
 	keyid = camel_partition_table_lookup(p->name_hash, name);
 	if (keyid != 0) {
+		/* TODO: We could just update the partition table's
+		   key pointer rather than having to delete it */
 		rb->deleted++;
 		camel_key_table_set_flags(p->name_index, keyid, 1, 1);
 		camel_partition_table_remove(p->name_hash, name);
@@ -549,7 +584,9 @@ text_index_add_name(CamelIndex *idx, const char *name)
 		rb->names++;
 	}
 
-	/* TODO: if keyid == 0, we had a failure, we shoudl somehow flag that, but for
+	camel_block_file_touch_block(p->blocks, p->blocks->root_block);
+
+	/* TODO: if keyid == 0, we had a failure, we should somehow flag that, but for
 	   now just return a valid object but discard its results, see text_index_write_name */
 
 	CAMEL_TEXT_INDEX_UNLOCK(idx, lock);
@@ -607,6 +644,7 @@ text_index_delete_name(CamelIndex *idx, const char *name)
 	keyid = camel_partition_table_lookup(p->name_hash, name);
 	if (keyid != 0) {
 		rb->deleted++;
+		camel_block_file_touch_block(p->blocks, p->blocks->root_block);
 		camel_key_table_set_flags(p->name_index, keyid, 1, 1);
 		camel_partition_table_remove(p->name_hash, name);
 	}
@@ -760,7 +798,6 @@ text_index_normalise(CamelIndex *idx, const char *in, void *data)
 	return word;
 }
 
-/* Need flags? */
 CamelTextIndex *
 camel_text_index_new(const char *path, int flags)
 {
@@ -789,24 +826,28 @@ camel_text_index_new(const char *path, int flags)
 		bl = camel_block_file_new_block(p->blocks);
 		rb->word_index_root = bl->id;
 		camel_block_file_unref_block(p->blocks, bl);
+		camel_block_file_touch_block(p->blocks, p->blocks->root_block);
 	}	
 
 	if (rb->word_hash_root == 0) {
 		bl = camel_block_file_new_block(p->blocks);
 		rb->word_hash_root = bl->id;
 		camel_block_file_unref_block(p->blocks, bl);
+		camel_block_file_touch_block(p->blocks, p->blocks->root_block);
 	}	
 
 	if (rb->name_index_root == 0) {
 		bl = camel_block_file_new_block(p->blocks);
 		rb->name_index_root = bl->id;
 		camel_block_file_unref_block(p->blocks, bl);
+		camel_block_file_touch_block(p->blocks, p->blocks->root_block);
 	}	
 
 	if (rb->name_hash_root == 0) {
 		bl = camel_block_file_new_block(p->blocks);
 		rb->name_hash_root = bl->id;
 		camel_block_file_unref_block(p->blocks, bl);
+		camel_block_file_touch_block(p->blocks, p->blocks->root_block);
 	}	
 
 	p->word_index = camel_key_table_new(p->blocks, rb->word_index_root);
@@ -1337,7 +1378,9 @@ text_index_cursor_reset(CamelIndexCursor *idc)
 	struct _CamelTextIndexCursorPrivate *p = CIC_PRIVATE(idc);
 
 	g_free(p->records);
+	p->records = NULL;
 	g_free(p->current);
+	p->current = NULL;
 	p->record_count = 0;
 	p->record_index = 0;
 	p->next = p->first;