Re-factor and cleanup:
authorSteve McIntyre <steve@einval.com>
Tue, 22 Nov 2011 18:41:45 +0000 (18:41 +0000)
committerSteve McIntyre <steve@einval.com>
Tue, 22 Nov 2011 18:41:45 +0000 (18:41 +0000)
 * never use free_results() inside an _unlocked function, make the
   lock and data safety tracking easier
 * factor out db_remove_size_entry_unlocked() and
   db_remove_cache_entry_unlocked() for easier use inside other
   database functions

C/fmdb.c

index e22df39..6cda1af 100644 (file)
--- a/C/fmdb.c
+++ b/C/fmdb.c
@@ -173,7 +173,6 @@ static int db_dump_size_unlocked(db_state_t *state)
     char sql_command[2 * PATH_MAX];
     int i = 0;
 
-    free_results();
     sprintf(sql_command, "SELECT * FROM sizes");
     error = sqlite3_exec(state->db, sql_command, results_callback, &result_type, &open_error);
     if (error)
@@ -203,7 +202,6 @@ int db_dump_size(FMDB *dbp)
 
     pthread_mutex_lock(&db_mutex);
     free_results();
-
     error = db_dump_size_unlocked(dbp);
     pthread_mutex_unlock(&db_mutex);
     return error;
@@ -217,6 +215,7 @@ static int db_count_size(FMDB *dbp)
     char *open_error;
     int result_type = RES_COUNT;
 
+    pthread_mutex_lock(&db_mutex);
     free_results();
 
     error = sqlite3_exec(state->db, "SELECT COUNT(*) FROM sizes;", results_callback, &result_type, &open_error);
@@ -226,6 +225,7 @@ static int db_count_size(FMDB *dbp)
                 __func__, error, open_error));
         if (open_error)
             sqlite3_free(open_error);
+        pthread_mutex_unlock(&db_mutex);
         return error;
     }
 
@@ -238,6 +238,7 @@ static int db_count_size(FMDB *dbp)
     else
         error = ENOENT;
 
+    pthread_mutex_unlock(&db_mutex);
     return error;
 }
 #endif
@@ -326,15 +327,13 @@ int db_lookup_size_entry(FMDB *dbp, const db_size_entry_t *in, db_size_entry_t *
     return error;
 }
 
-int db_remove_size_entry(FMDB *dbp, const db_size_entry_t *in)
+static int db_remove_size_entry_unlocked(FMDB *dbp, const db_size_entry_t *in)
 {
     int error = 0;
     db_state_t *state = dbp;
     char *open_error;
     char sql_command[2 * PATH_MAX];
 
-    pthread_mutex_lock(&db_mutex);
-    free_results();
     sprintf(sql_command,
             "DELETE FROM sizes WHERE flac_path == '%s' AND format_choice == '%s' AND format_quality == %s;",
             in->flac_path, in->format_choice, in->format_quality);
@@ -346,6 +345,15 @@ int db_remove_size_entry(FMDB *dbp, const db_size_entry_t *in)
         if (open_error)
             sqlite3_free(open_error);
     }
+    return error;
+}
+
+int db_remove_size_entry(FMDB *dbp, const db_size_entry_t *in)
+{
+    int error = 0;
+
+    pthread_mutex_lock(&db_mutex);
+    error = db_remove_size_entry_unlocked(dbp, in);
     pthread_mutex_unlock(&db_mutex);
     return error;
 }
@@ -415,8 +423,6 @@ static int db_dump_cache_unlocked(db_state_t *state, long long *size)
     int i = 0;
     long long total_size = 0;
 
-    free_results();
-
     sprintf(sql_command, "SELECT * FROM cache");
     error = sqlite3_exec(state->db, sql_command, results_callback, &result_type, &open_error);
     if (error)
@@ -450,7 +456,6 @@ int db_dump_cache(FMDB *dbp, long long *size)
 
     pthread_mutex_lock(&db_mutex);
     free_results();
-
     error = db_dump_cache_unlocked(dbp, size);
     pthread_mutex_unlock(&db_mutex);
     return error;
@@ -465,8 +470,6 @@ static int _db_cache_space_used_unlocked(FMDB *dbp, long long *size)
     char sql_command[2 * PATH_MAX];
     long long total_size = 0;
 
-    free_results();
-
     sprintf(sql_command, "SELECT * FROM cache");
     error = sqlite3_exec(state->db, sql_command, results_callback, &result_type, &open_error);
     if (error)
@@ -503,8 +506,6 @@ static int db_count_cache(FMDB *dbp, int *num_entries)
     char *open_error;
     int result_type = RES_COUNT;
 
-    free_results();
-
     error = sqlite3_exec(state->db, "SELECT COUNT(*) FROM cache;", results_callback, &result_type, &open_error);
     if (error)
     {
@@ -570,6 +571,7 @@ int db_store_cache_entry(FMDB *dbp, const db_cache_entry_t *in)
         pthread_mutex_unlock(&db_mutex);
         return error;
     }
+    free_results();
     _db_cache_space_used_unlocked(state, &junk);
     pthread_mutex_unlock(&db_mutex);
     return error;
@@ -588,6 +590,7 @@ int db_lookup_cache_entry_by_flac_path(FMDB *dbp, const db_cache_entry_t *in, db
 
     pthread_mutex_lock(&db_mutex);
 #ifdef DB_COUNT_DEBUG
+    free_results();
     db_count_cache(dbp, &num_entries);
 #endif
     free_results();
@@ -635,6 +638,7 @@ int db_lookup_cache_entry_by_cache_path(FMDB *dbp, const db_cache_entry_t *in, d
 
     pthread_mutex_lock(&db_mutex);
 #ifdef DB_COUNT_DEBUG
+    free_results();
     db_count_cache(dbp, &num_entries);
 #endif
     free_results();
@@ -680,6 +684,7 @@ int db_lookup_cache_entry_by_lru(FMDB *dbp, db_cache_entry_t *out, int *num)
     long long junk;
 
     pthread_mutex_lock(&db_mutex);
+    free_results();
     db_count_cache(dbp, &num_entries);
     free_results();
 
@@ -712,6 +717,7 @@ int db_lookup_cache_entry_by_lru(FMDB *dbp, db_cache_entry_t *out, int *num)
         DBLOG0((logfile, "%s: failed to find an LRU entry\n", __func__));
         error = ENOENT;
         *num = num_entries;
+        free_results();
         db_dump_cache_unlocked(dbp, &junk);
     }
     
@@ -719,7 +725,7 @@ int db_lookup_cache_entry_by_lru(FMDB *dbp, db_cache_entry_t *out, int *num)
     return error;
 }
 
-int db_remove_cache_entry(FMDB *dbp, const db_cache_entry_t *in)
+static int db_remove_cache_entry_unlocked(FMDB *dbp, const db_cache_entry_t *in)
 {
     int error = 0;
     db_state_t *state = dbp;
@@ -727,7 +733,6 @@ int db_remove_cache_entry(FMDB *dbp, const db_cache_entry_t *in)
     char sql_command[2 * PATH_MAX];
     long long junk;
 
-    pthread_mutex_lock(&db_mutex);
     sprintf(sql_command,
             "DELETE FROM cache WHERE flac_path == '%s';",
             in->flac_path);
@@ -739,7 +744,16 @@ int db_remove_cache_entry(FMDB *dbp, const db_cache_entry_t *in)
         if (open_error)
             sqlite3_free(open_error);
     }
+    free_results();
     _db_cache_space_used_unlocked(state, &junk);
+    return error;
+}
+
+int db_remove_cache_entry(FMDB *dbp, const db_cache_entry_t *in)
+{
+    int error = 0;
+    pthread_mutex_lock(&db_mutex);
+    error = db_remove_cache_entry_unlocked(dbp, in);
     pthread_mutex_unlock(&db_mutex);
     return error;
 }
@@ -763,9 +777,11 @@ int sync_cache_and_db(FMDB *dbp, char *cache_dir)
      * that don't, or that look wrong: delete the DB entry */
     DBLOG1((logfile, "%s: Checking DB entry sanity\n", __func__));
 #ifdef DB_COUNT_DEBUG
+    free_results();
     db_count_cache(dbp, &num_entries);
 #endif
 #ifdef DB_COUNT_DEBUG
+    free_results();
     db_count_size(dbp);
 #endif
     free_results();
@@ -806,7 +822,8 @@ int sync_cache_and_db(FMDB *dbp, char *cache_dir)
             {
                 db_remove_cache_entry(dbp, entry);
                 removed_db_entries++;
-                entry_changed++;            }
+                entry_changed++;
+            }
             else
             {
                 
@@ -832,7 +849,7 @@ int sync_cache_and_db(FMDB *dbp, char *cache_dir)
                 {
                     DBLOG1((logfile, "%s: can't stat cache file %s, error %d. Deleting DB entry\n",
                             __func__, cache_file_name, errno));
-                    db_remove_cache_entry(dbp, entry);
+                    db_remove_cache_entry_unlocked(dbp, entry);
                     removed_db_entries++;
                     entry_changed++;
                 }
@@ -916,6 +933,7 @@ FMDB *db_open(char *db_name)
             }
         }
     }
+    free_results();
     _db_cache_space_used_unlocked(dbp, &junk);
     return dbp;
 }