Merge pull request #1244 from pi-hole/tweak/dbclose

Ensure dbclose() is not called multiple times

src/daemon.c

@@ -18,8 +18,6 @@
 #include "api/socket.h"
 // gravityDB_close()
 #include "database/gravity-db.h"
-// dbclose()
-#include "database/common.h"
 // destroy_shmem()
 #include "shmem.h"
 // uname()

src/database/aliasclients.c

@@ -259,8 +259,8 @@ void reset_aliasclient(sqlite3 *db, clientsData *client)
 	// Find corresponding alias-client (if any)
 	client->aliasclient_id = get_aliasclient_ID(db, client);
 
-	if(db_opened)
-		dbclose(&db);
+	// Close the database if we opened it here
+	if(db_opened) dbclose(&db);
 
 	// Skip if there is no responsible alias-client
 	if(client->aliasclient_id == -1)
@@ -360,6 +360,5 @@ void reimport_aliasclients(sqlite3 *db)
 	}
 
 	// Close the database if we opened it here
-	if(db_opened)
-		dbclose(&db);
+	if(db_opened) dbclose(&db);
 }

src/database/common.c

@@ -149,7 +149,6 @@ int dbquery(sqlite3* db, const char *format, ...)
 		logg("ERROR: SQL query \"%s\" failed: %s",
 		     query, sqlite3_errstr(rc));
 		sqlite3_free(query);
-		dbclose(&db);
 		checkFTLDBrc(rc);
 		return rc;
 	}
@@ -469,7 +468,6 @@ bool db_set_counter(sqlite3 *db, const enum counters_table_props ID, const long
 	if(rc != SQLITE_OK)
 	{
 		checkFTLDBrc(rc);
-		dbclose(&db);
 		return false;
 	}
 
@@ -482,7 +480,6 @@ bool db_update_counters(sqlite3 *db, const int total, const int blocked)
 	if(rc != SQLITE_OK)
 	{
 		checkFTLDBrc(rc);
-		dbclose(&db);
 		return false;
 	}
 
@@ -490,7 +487,6 @@ bool db_update_counters(sqlite3 *db, const int total, const int blocked)
 	if(rc != SQLITE_OK)
 	{
 		checkFTLDBrc(rc);
-		dbclose(&db);
 		return false;
 	}
 
@@ -563,7 +559,6 @@ long int get_max_query_ID(sqlite3 *db)
 		{
 			logg("Encountered prepare error in get_max_query_ID(): %s", sqlite3_errstr(rc));
 			checkFTLDBrc(rc);
-			dbclose(&db);
 		}
 
 		// Return okay if the database is busy
@@ -575,7 +570,6 @@ long int get_max_query_ID(sqlite3 *db)
 	{
 		logg("Encountered step error in get_max_query_ID(): %s", sqlite3_errstr(rc));
 		checkFTLDBrc(rc);
-		dbclose(&db);
 		return DB_FAILED;
 	}
 

src/database/message-table.c

@@ -125,6 +125,7 @@ bool flush_message_table(void)
 static bool add_message(enum message_type type,
                         const char *message, const int count,...)
 {
+	bool okay = false;
 	// Return early if database is known to be broken
 	if(FTLDBerror())
 		return false;
@@ -146,8 +147,7 @@ static bool add_message(enum message_type type,
 		if( rc != SQLITE_OK ){
 			logg("add_message(type=%u, message=%s) - SQL error prepare DELETE: %s",
 			     type, message, sqlite3_errstr(rc));
-			dbclose(&db);
-			return false;
+			goto end_of_add_message;
 		}
 
 		// Bind type to prepared statement
@@ -157,8 +157,7 @@ static bool add_message(enum message_type type,
 			     type, message, sqlite3_errstr(rc));
 			sqlite3_reset(stmt);
 			sqlite3_finalize(stmt);
-			dbclose(&db);
-			return false;
+			goto end_of_add_message;
 		}
 
 		// Bind message to prepared statement
@@ -168,8 +167,7 @@ static bool add_message(enum message_type type,
 			     type, message, sqlite3_errstr(rc));
 			sqlite3_reset(stmt);
 			sqlite3_finalize(stmt);
-			dbclose(&db);
-			return false;
+			goto end_of_add_message;
 		}
 
 		// Execute and finalize
@@ -177,8 +175,7 @@ static bool add_message(enum message_type type,
 		{
 			logg("add_message(type=%u, message=%s) - SQL error step DELETE: %s",
 			     type, message, sqlite3_errstr(rc));
-			dbclose(&db);
-			return false;
+			goto end_of_add_message;
 		}
 		sqlite3_clear_bindings(stmt);
 		sqlite3_reset(stmt);
@@ -194,8 +191,7 @@ static bool add_message(enum message_type type,
 	{
 		logg("add_message(type=%u, message=%s) - SQL error prepare: %s",
 		     type, message, sqlite3_errstr(rc));
-		dbclose(&db);
-		return false;
+		goto end_of_add_message;
 	}
 
 	// Bind type to prepared statement
@@ -205,8 +201,7 @@ static bool add_message(enum message_type type,
 		     type, message, sqlite3_errstr(rc));
 		sqlite3_reset(stmt);
 		sqlite3_finalize(stmt);
-		dbclose(&db);
-		return false;
+		goto end_of_add_message;
 	}
 
 	// Bind message to prepared statement
@@ -216,8 +211,7 @@ static bool add_message(enum message_type type,
 		     type, message, sqlite3_errstr(rc));
 		sqlite3_reset(stmt);
 		sqlite3_finalize(stmt);
-		dbclose(&db);
-		return false;
+		goto end_of_add_message;
 	}
 
 	va_list ap;
@@ -249,9 +243,8 @@ static bool add_message(enum message_type type,
 			sqlite3_reset(stmt);
 			sqlite3_finalize(stmt);
 			checkFTLDBrc(rc);
-			dbclose(&db);
 			va_end(ap);
-			return false;
+			goto end_of_add_message;
 		}
 	}
 	va_end(ap);
@@ -263,18 +256,19 @@ static bool add_message(enum message_type type,
 	{
 		logg("Encountered error while trying to store message in long-term database: %s", sqlite3_errstr(rc));
 		checkFTLDBrc(rc);
-		dbclose(&db);
-		return false;
+		goto end_of_add_message;
 	}
 
+	// Final database handling
 	sqlite3_clear_bindings(stmt);
 	sqlite3_reset(stmt);
 	sqlite3_finalize(stmt);
+	okay = true;
 
-	// Close database connection
+end_of_add_message: // Close database connection
 	dbclose(&db);
 
-	return true;
+	return okay;
 }
 
 void logg_regex_warning(const char *type, const char *warning, const int dbindex, const char *regex)

src/database/network-table.c

@@ -1854,8 +1854,9 @@ char *__attribute__((malloc)) getMACfromIP(sqlite3* db, const char *ipaddr)
 		logg("getMACfromIP(\"%s\") - SQL error prepare: %s",
 		     ipaddr, sqlite3_errstr(rc));
 		checkFTLDBrc(rc);
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return NULL;
 	}
 
@@ -1867,8 +1868,9 @@ char *__attribute__((malloc)) getMACfromIP(sqlite3* db, const char *ipaddr)
 		checkFTLDBrc(rc);
 		sqlite3_reset(stmt);
 		sqlite3_finalize(stmt);
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return NULL;
 	}
 
@@ -1899,8 +1901,7 @@ char *__attribute__((malloc)) getMACfromIP(sqlite3* db, const char *ipaddr)
 	sqlite3_reset(stmt);
 	sqlite3_finalize(stmt);
 
-	if(db_opened)
-		dbclose(&db);
+	if(db_opened) dbclose(&db);
 
 	return hwaddr;
 }
@@ -1941,8 +1942,9 @@ int getAliasclientIDfromIP(sqlite3 *db, const char *ipaddr)
 		logg("getAliasclientIDfromIP(\"%s\") - SQL error prepare: %s",
 		     ipaddr, sqlite3_errstr(rc));
 		checkFTLDBrc(rc);
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return DB_FAILED;
 	}
 
@@ -1954,8 +1956,9 @@ int getAliasclientIDfromIP(sqlite3 *db, const char *ipaddr)
 		checkFTLDBrc(rc);
 		sqlite3_reset(stmt);
 		sqlite3_finalize(stmt);
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return DB_FAILED;
 	}
 
@@ -1980,8 +1983,8 @@ int getAliasclientIDfromIP(sqlite3 *db, const char *ipaddr)
 	// Finalize statement and close database handle
 	sqlite3_reset(stmt);
 	sqlite3_finalize(stmt);
-	if(db_opened)
-		dbclose(&db);
+
+	if(db_opened) dbclose(&db);
 
 	return aliasclient_id;
 }
@@ -2024,8 +2027,9 @@ char *__attribute__((malloc)) getNameFromIP(sqlite3 *db, const char *ipaddr)
 		logg("getNameFromIP(\"%s\") - SQL error prepare: %s",
 		     ipaddr, sqlite3_errstr(rc));
 		checkFTLDBrc(rc);
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return NULL;
 	}
 
@@ -2037,8 +2041,9 @@ char *__attribute__((malloc)) getNameFromIP(sqlite3 *db, const char *ipaddr)
 		checkFTLDBrc(rc);
 		sqlite3_reset(stmt);
 		sqlite3_finalize(stmt);
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return NULL;
 	}
 
@@ -2066,8 +2071,8 @@ char *__attribute__((malloc)) getNameFromIP(sqlite3 *db, const char *ipaddr)
 	// Return here if we found the name
 	if(name != NULL)
 	{
-		if(db_opened)
-			dbclose(&db);
+		if(db_opened) dbclose(&db);
+
 		return name;
 	}
 
@@ -2083,8 +2088,9 @@ char *__attribute__((malloc)) getNameFromIP(sqlite3 *db, const char *ipaddr)
 	{
 		logg("getNameFromIP(\"%s\") - SQL error prepare: %s",
 		     ipaddr, sqlite3_errstr(rc));
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return NULL;
 	}
 
@@ -2096,8 +2102,9 @@ char *__attribute__((malloc)) getNameFromIP(sqlite3 *db, const char *ipaddr)
 		checkFTLDBrc(rc);
 		sqlite3_reset(stmt);
 		sqlite3_finalize(stmt);
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return NULL;
 	}
 
@@ -2127,8 +2134,7 @@ char *__attribute__((malloc)) getNameFromIP(sqlite3 *db, const char *ipaddr)
 	sqlite3_reset(stmt);
 	sqlite3_finalize(stmt);
 
-	if(db_opened)
-		dbclose(&db);
+	if(db_opened) dbclose(&db);
 
 	return name;
 }
@@ -2167,8 +2173,9 @@ char *__attribute__((malloc)) getIfaceFromIP(sqlite3 *db, const char *ipaddr)
 	{
 		logg("getIfaceFromIP(\"%s\") - SQL error prepare: %s",
 		     ipaddr, sqlite3_errstr(rc));
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return NULL;
 	}
 
@@ -2186,8 +2193,9 @@ char *__attribute__((malloc)) getIfaceFromIP(sqlite3 *db, const char *ipaddr)
 		checkFTLDBrc(rc);
 		sqlite3_reset(stmt);
 		sqlite3_finalize(stmt);
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return NULL;
 	}
 
@@ -2212,8 +2220,7 @@ char *__attribute__((malloc)) getIfaceFromIP(sqlite3 *db, const char *ipaddr)
 	sqlite3_reset(stmt);
 	sqlite3_finalize(stmt);
 
-	if(db_opened)
-		dbclose(&db);
+	if(db_opened) dbclose(&db);
 
 	return iface;
 }

src/database/query-table.c

@@ -51,8 +51,7 @@ int get_number_of_queries_in_DB(sqlite3 *db)
 	// Count number of rows using the index timestamp is faster than select(*)
 	int result = db_query_int(db, "SELECT COUNT(timestamp) FROM queries");
 
-	if(db_opened)
-		dbclose(&db);
+	if(db_opened) dbclose(&db);
 
 	return result;
 }
@@ -96,8 +95,9 @@ int DB_save_queries(sqlite3 *db)
 
 		logg("%s: Storing queries in long-term database failed: %s", text, sqlite3_errstr(rc));
 		checkFTLDBrc(rc);
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return DB_FAILED;
 	}
 
@@ -121,8 +121,9 @@ int DB_save_queries(sqlite3 *db)
 		if(!checkFTLDBrc(rc))
 			logg("%s  Keeping queries in memory for later new attempt", spaces);
 		saving_failed_before = true;
-		if(db_opened)
-			dbclose(&db);
+
+		if(db_opened) dbclose(&db);
+
 		return DB_FAILED;
 	}
 
@@ -274,8 +275,7 @@ int DB_save_queries(sqlite3 *db)
 			saving_failed_before = true;
 		}
 
-		if(db_opened)
-			dbclose(&db);
+		if(db_opened) dbclose(&db);
 
 		return DB_FAILED;
 	}
@@ -301,8 +301,7 @@ int DB_save_queries(sqlite3 *db)
 			saving_failed_before = true;
 		}
 
-		if(db_opened)
-			dbclose(&db);
+		if(db_opened) dbclose(&db);
 
 		return DB_FAILED;
 	}
@@ -318,8 +317,7 @@ int DB_save_queries(sqlite3 *db)
 		}
 	}
 
-	if(db_opened)
-		dbclose(&db);
+	if(db_opened) dbclose(&db);
 
 	return saved;
 }
@@ -387,8 +385,7 @@ void DB_read_queries(void)
 	if( rc != SQLITE_OK ){
 		logg("DB_read_queries() - SQL error prepare: %s", sqlite3_errstr(rc));
 		checkFTLDBrc(rc);
-		dbclose(&db);
-		return;
+		goto end_of_DB_read_queries;
 	}
 
 	// Bind limit
@@ -396,8 +393,7 @@ void DB_read_queries(void)
 	{
 		logg("DB_read_queries() - Failed to bind type mintime: %s", sqlite3_errstr(rc));
 		checkFTLDBrc(rc);
-		dbclose(&db);
-		return;
+		goto end_of_DB_read_queries;
 	}
 
 	// Lock shared memory
@@ -638,13 +634,12 @@ void DB_read_queries(void)
 	if( rc != SQLITE_DONE ){
 		logg("DB_read_queries() - SQL error step: %s", sqlite3_errstr(rc));
 		checkFTLDBrc(rc);
-		dbclose(&db);
-		return;
+		goto end_of_DB_read_queries;
 	}
 
 	// Finalize SQLite3 statement
 	sqlite3_finalize(stmt);
 
-	// Close database here, we have to reopen it later (after forking)
+end_of_DB_read_queries:	// Close database here, we have to reopen it later (after forking)
 	dbclose(&db);
 }