[tracker/binary-log-2: 32/63] libtracker-db: Fixed Philip's comments:

[Jürg Billeter <juergbi src gnome org>]

commit d7a87831a8622a47803c25a3e2a8f89645102d96
Author: Martyn Russell <martyn lanedo com>
Date:   Mon Jan 4 19:32:34 2010 +0000

    libtracker-db: Fixed Philip's comments:
    
    - Revert writer variables to static vars (don't alloc)
    - Don't use g_slice for TrackerDBJournalEntry, use g_new instead.
    - the g_assert(memcpy()...) for the journal header was removed because
      it failed, this has been re-added using: g_assert_cmpint for each
      character and it works.

 src/libtracker-db/tracker-db-journal.c |  219 ++++++++++++++++----------------
 1 files changed, 110 insertions(+), 109 deletions(-)
---
diff --git a/src/libtracker-db/tracker-db-journal.c b/src/libtracker-db/tracker-db-journal.c
index e12a7ac..46ffd10 100644
--- a/src/libtracker-db/tracker-db-journal.c
+++ b/src/libtracker-db/tracker-db-journal.c
@@ -56,21 +56,15 @@ struct TrackerDBJournalEntry {
 	const gchar *object;
 };
 
-typedef struct TrackerDBJournal TrackerDBJournal;
+static gchar *journal_filename = NULL;
+static FILE *journal = NULL;
 
-struct TrackerDBJournal {
-	gchar *filename;
-	FILE *file;
-
-	gsize cur_size;
-	guint cur_block_len;
-	guint cur_block_alloc;
-	gchar *cur_block;
-	guint cur_entry_amount;
-	guint cur_pos;
-};
-
-static TrackerDBJournal *journal = NULL;
+static gsize cur_size = 0;
+static guint cur_block_len = 0;
+static guint cur_block_alloc = 0;
+static gchar *cur_block = NULL;
+static guint cur_entry_amount = 0;
+static guint cur_pos = 0;
 
 static guint32
 read_uint32 (const gchar *data)
@@ -93,26 +87,26 @@ nearest_pow (gint num)
 static void
 cur_block_maybe_expand (guint len)
 {
-	guint want_alloc = journal->cur_block_len + len;
+	guint want_alloc = cur_block_len + len;
 
-	if (want_alloc > journal->cur_block_alloc) {
+	if (want_alloc > cur_block_alloc) {
 		want_alloc = nearest_pow (want_alloc);
 		want_alloc = MAX (want_alloc, MIN_BLOCK_SIZE);
-		journal->cur_block = g_realloc (journal->cur_block, want_alloc);
-		journal->cur_block_alloc = want_alloc;
+		cur_block = g_realloc (cur_block, want_alloc);
+		cur_block_alloc = want_alloc;
 	}
 }
 
 static void
 cur_block_kill (void)
 {
-	journal->cur_block_len = 0;
-	journal->cur_pos = 0;
-	journal->cur_entry_amount = 0;
-	journal->cur_block_alloc = 0;
+	cur_block_len = 0;
+	cur_pos = 0;
+	cur_entry_amount = 0;
+	cur_block_alloc = 0;
 
-	g_free (journal->cur_block);
-	journal->cur_block = NULL;
+	g_free (cur_block);
+	cur_block = NULL;
 }
 
 static void
@@ -150,45 +144,43 @@ tracker_db_journal_init (const gchar *filename)
 
 	g_return_val_if_fail (journal == NULL, FALSE);
 
-	journal = g_new0 (TrackerDBJournal, 1);
-
 	/* Used mostly for testing */
 	if (G_UNLIKELY (filename)) {
-		journal->filename = g_strdup (filename);
+		journal_filename = g_strdup (filename);
 	} else {
-		journal->filename = g_build_filename (g_get_user_data_dir (),
-		                                      "tracker",
-		                                      "data",
-		                                      JOURNAL_FILENAME,
-		                                      NULL);
+		journal_filename = g_build_filename (g_get_user_data_dir (),
+		                                     "tracker",
+		                                     "data",
+		                                     JOURNAL_FILENAME,
+		                                     NULL);
 	}
 
-	journal->file = g_fopen (journal->filename, "a");
+	journal = g_fopen (journal_filename, "a");
 
-	if (g_stat (journal->filename, &st) == 0) {
-		journal->cur_size = (gsize) st.st_size;
+	if (g_stat (journal_filename, &st) == 0) {
+		cur_size = (gsize) st.st_size;
 	}
 
-	if (journal->cur_size == 0) {
-		g_assert (journal->cur_block_len == 0);
-		g_assert (journal->cur_block_alloc == 0);
-		g_assert (journal->cur_block == NULL);
-		g_assert (journal->cur_block == NULL);
+	if (cur_size == 0) {
+		g_assert (cur_block_len == 0);
+		g_assert (cur_block_alloc == 0);
+		g_assert (cur_block == NULL);
+		g_assert (cur_block == NULL);
 
 		cur_block_maybe_expand (8);
 
-		journal->cur_block[0] = 't';
-		journal->cur_block[1] = 'r';
-		journal->cur_block[2] = 'l';
-		journal->cur_block[3] = 'o';
-		journal->cur_block[4] = 'g';
-		journal->cur_block[5] = '\0';
-		journal->cur_block[6] = '0';
-		journal->cur_block[7] = '1';
+		cur_block[0] = 't';
+		cur_block[1] = 'r';
+		cur_block[2] = 'l';
+		cur_block[3] = 'o';
+		cur_block[4] = 'g';
+		cur_block[5] = '\0';
+		cur_block[6] = '0';
+		cur_block[7] = '1';
 
-		write (fileno (journal->file), journal->cur_block, 8);
+		write (fileno (journal), cur_block, 8);
 
-		journal->cur_size += 8;
+		cur_size += 8;
 
 		cur_block_kill ();
 	}
@@ -201,14 +193,13 @@ tracker_db_journal_shutdown (void)
 {
 	g_return_val_if_fail (journal != NULL, FALSE);
 
-	if (journal->file) {
-		fclose (journal->file);
+	if (journal) {
+		fclose (journal);
+		journal = NULL;
 	}
 
-	g_free (journal->filename);
-
-	g_free (journal);
-	journal = NULL;
+	g_free (journal_filename);
+	journal_filename = NULL;
 
 	return TRUE;
 }
@@ -218,7 +209,7 @@ tracker_db_journal_get_size (void)
 {
 	g_return_val_if_fail (journal != NULL, FALSE);
 
-	return journal->cur_size;
+	return cur_size;
 }
 
 const gchar *
@@ -226,7 +217,7 @@ tracker_db_journal_get_filename (void)
 {
 	g_return_val_if_fail (journal != NULL, FALSE);
 
-	return (const gchar*) journal->filename;
+	return (const gchar*) journal_filename;
 }
 
 gboolean
@@ -243,10 +234,10 @@ tracker_db_journal_start_transaction (void)
 	 * Check and keep in sync the offset variable at
 	 * tracker_db_journal_commit_transaction too */
 
-	memset (journal->cur_block, 0, size);
+	memset (cur_block, 0, size);
 
-	journal->cur_pos = journal->cur_block_len = size;
-	journal->cur_entry_amount = 0;
+	cur_pos = cur_block_len = size;
+	cur_entry_amount = 0;
 
 	return TRUE;
 }
@@ -268,13 +259,13 @@ tracker_db_journal_append_delete_statement (guint32      s_code,
 
 	cur_block_maybe_expand (size);
 
-	cur_setnum (journal->cur_block, &journal->cur_pos, data_format);
-	cur_setnum (journal->cur_block, &journal->cur_pos, s_code);
-	cur_setnum (journal->cur_block, &journal->cur_pos, p_code);
-	cur_setstr (journal->cur_block, &journal->cur_pos, object, o_len);
+	cur_setnum (cur_block, &cur_pos, data_format);
+	cur_setnum (cur_block, &cur_pos, s_code);
+	cur_setnum (cur_block, &cur_pos, p_code);
+	cur_setstr (cur_block, &cur_pos, object, o_len);
 
-	journal->cur_entry_amount++;
-	journal->cur_block_len += size;
+	cur_entry_amount++;
+	cur_block_len += size;
 
 	return TRUE;
 }
@@ -294,13 +285,13 @@ tracker_db_journal_append_delete_statement_code (guint32 s_code,
 
 	cur_block_maybe_expand (size);
 
-	cur_setnum (journal->cur_block, &journal->cur_pos, data_format);
-	cur_setnum (journal->cur_block, &journal->cur_pos, s_code);
-	cur_setnum (journal->cur_block, &journal->cur_pos, p_code);
-	cur_setnum (journal->cur_block, &journal->cur_pos, o_code);
+	cur_setnum (cur_block, &cur_pos, data_format);
+	cur_setnum (cur_block, &cur_pos, s_code);
+	cur_setnum (cur_block, &cur_pos, p_code);
+	cur_setnum (cur_block, &cur_pos, o_code);
 
-	journal->cur_entry_amount++;
-	journal->cur_block_len += size;
+	cur_entry_amount++;
+	cur_block_len += size;
 
 	return TRUE;
 }
@@ -322,13 +313,13 @@ tracker_db_journal_append_insert_statement (guint32      s_code,
 
 	cur_block_maybe_expand (size);
 
-	cur_setnum (journal->cur_block, &journal->cur_pos, data_format);
-	cur_setnum (journal->cur_block, &journal->cur_pos, s_code);
-	cur_setnum (journal->cur_block, &journal->cur_pos, p_code);
-	cur_setstr (journal->cur_block, &journal->cur_pos, object, o_len);
+	cur_setnum (cur_block, &cur_pos, data_format);
+	cur_setnum (cur_block, &cur_pos, s_code);
+	cur_setnum (cur_block, &cur_pos, p_code);
+	cur_setstr (cur_block, &cur_pos, object, o_len);
 
-	journal->cur_entry_amount++;
-	journal->cur_block_len += size;
+	cur_entry_amount++;
+	cur_block_len += size;
 
 	return TRUE;
 }
@@ -348,13 +339,13 @@ tracker_db_journal_append_insert_statement_code (guint32 s_code,
 
 	cur_block_maybe_expand (size);
 
-	cur_setnum (journal->cur_block, &journal->cur_pos, data_format);
-	cur_setnum (journal->cur_block, &journal->cur_pos, s_code);
-	cur_setnum (journal->cur_block, &journal->cur_pos, p_code);
-	cur_setnum (journal->cur_block, &journal->cur_pos, o_code);
+	cur_setnum (cur_block, &cur_pos, data_format);
+	cur_setnum (cur_block, &cur_pos, s_code);
+	cur_setnum (cur_block, &cur_pos, p_code);
+	cur_setnum (cur_block, &cur_pos, o_code);
 
-	journal->cur_entry_amount++;
-	journal->cur_block_len += size;
+	cur_entry_amount++;
+	cur_block_len += size;
 
 	return TRUE;
 }
@@ -375,12 +366,12 @@ tracker_db_journal_append_resource (guint32      s_code,
 
 	cur_block_maybe_expand (size);
 
-	cur_setnum (journal->cur_block, &journal->cur_pos, data_format);
-	cur_setnum (journal->cur_block, &journal->cur_pos, s_code);
-	cur_setstr (journal->cur_block, &journal->cur_pos, uri, o_len);
+	cur_setnum (cur_block, &cur_pos, data_format);
+	cur_setnum (cur_block, &cur_pos, s_code);
+	cur_setstr (cur_block, &cur_pos, uri, o_len);
 
-	journal->cur_entry_amount++;
-	journal->cur_block_len += size;
+	cur_entry_amount++;
+	cur_block_len += size;
 
 	return TRUE;
 }
@@ -411,25 +402,25 @@ tracker_db_journal_commit_transaction (void)
 
 	cur_block_maybe_expand (size);
 
-	cur_setnum (journal->cur_block, &begin_pos, journal->cur_block_len);
-	cur_setnum (journal->cur_block, &begin_pos, journal->cur_entry_amount);
+	cur_setnum (cur_block, &begin_pos, cur_block_len);
+	cur_setnum (cur_block, &begin_pos, cur_entry_amount);
 
-	cur_setnum (journal->cur_block, &journal->cur_pos, journal->cur_block_len);
+	cur_setnum (cur_block, &cur_pos, cur_block_len);
 
-	journal->cur_block_len += size;
+	cur_block_len += size;
 
 	/* CRC is calculated from entries until appended amount int */
 
-	crc = tracker_crc32 (journal->cur_block + offset, journal->cur_block_len - offset);
-	cur_setnum (journal->cur_block, &begin_pos, crc);
+	crc = tracker_crc32 (cur_block + offset, cur_block_len - offset);
+	cur_setnum (cur_block, &begin_pos, crc);
 
 	/* FIXME: What if we don't write all of len, needs improving. */
-	if (write (fileno (journal->file), journal->cur_block, journal->cur_block_len) == -1) {
+	if (write (fileno (journal), cur_block, cur_block_len) == -1) {
 		g_critical ("Could not write to journal, %s", g_strerror (errno));
 		return FALSE;
 	}
 
-	journal->cur_size += journal->cur_block_len;
+	cur_size += cur_block_len;
 	cur_block_kill ();
 
 	return TRUE;
@@ -440,7 +431,7 @@ tracker_db_journal_fsync (void)
 {
 	g_return_val_if_fail (journal != NULL, FALSE);
 
-	return fsync (fileno (journal->file)) == 0;
+	return fsync (fileno (journal)) == 0;
 }
 
 /*
@@ -451,23 +442,23 @@ tracker_db_journal_entry_new (const gchar *filename)
 {
 	TrackerDBJournalEntry *entry;
 	GError *error = NULL;
-	gchar *journal_filename;
+	gchar *filename_used;
 
 	/* Used mostly for testing */
 	if (G_UNLIKELY (filename)) {
-		journal_filename = g_strdup (filename);
+		filename_used = g_strdup (filename);
 	} else {
-		journal_filename = g_build_filename (g_get_user_data_dir (),
-		                                     "tracker",
-		                                     "data",
-		                                     JOURNAL_FILENAME,
-		                                     NULL);
+		filename_used = g_build_filename (g_get_user_data_dir (),
+		                                  "tracker",
+		                                  "data",
+		                                  JOURNAL_FILENAME,
+		                                  NULL);
 	}
 
-	entry = g_slice_new0 (TrackerDBJournalEntry);
+	entry = g_new0 (TrackerDBJournalEntry, 1);
 
 	entry->type = TRACKER_DB_JOURNAL_START;
-	entry->filename = journal_filename;
+	entry->filename = filename_used;
 
 	entry->file = g_mapped_file_new (entry->filename, FALSE, &error);
 
@@ -486,6 +477,16 @@ tracker_db_journal_entry_new (const gchar *filename)
 
 	/* verify journal file header */
 	g_assert (entry->end - entry->current >= 8);
+
+	g_assert_cmpint (entry->current[0], ==, 't');
+	g_assert_cmpint (entry->current[1], ==, 'r');
+	g_assert_cmpint (entry->current[2], ==, 'l');
+	g_assert_cmpint (entry->current[3], ==, 'o');
+	g_assert_cmpint (entry->current[4], ==, 'g');
+	g_assert_cmpint (entry->current[5], ==, '\0');
+	g_assert_cmpint (entry->current[6], ==, '0');
+	g_assert_cmpint (entry->current[7], ==, '1');
+
 	entry->current += 8;
 
 	return entry;
@@ -506,7 +507,7 @@ tracker_db_journal_entry_free (TrackerDBJournalEntry *entry)
 
 	g_free (entry->filename);
 
-	g_slice_free (TrackerDBJournalEntry, entry);
+	g_free (entry);
 
 	return TRUE;
 }