Large-scale overhaul for the long-term database handling.

Signed-off-by: DL6ER <dl6er@dl6er.de>

src/database/common.c

@@ -18,72 +18,58 @@
 #include "timers.h"
 #include "files.h"
 
-sqlite3 *FTL_db;
-// This boolean is set to false once we hit the
-// first database error to prevent further access
-// to the pihole-FTL.db database
+sqlite3 *FTL_db = NULL;
 bool database = true;
 bool DBdeleteoldqueries = false;
 long int lastdbindex = 0;
 
 static pthread_mutex_t dblock;
 
-bool check_database(int rc)
-{
-	// We will retry if the database is busy at the moment
-	// However, we won't retry if any other error happened
-	// and - instead - disable the database functionality
-	// altogether in FTL (setting database to false)
-	if(rc != SQLITE_OK &&
-	   rc != SQLITE_DONE &&
-	   rc != SQLITE_ROW &&
-	   rc != SQLITE_BUSY)
-	{
-		logg("check_database(%i): Disabling database connection due to error", rc);
-		dbclose();
-		database = false;
-	}
-
-	return database;
-}
-
 void dbclose(void)
 {
-	int rc = SQLITE_OK;
-
 	// Only try to close an existing database connection
-	if(FTL_db != NULL)
+	int rc = SQLITE_OK;
+	if( FTL_db != NULL )
+	{
 		rc = sqlite3_close(FTL_db);
+		FTL_db = NULL;
+	}
 
 	// Report any error
 	if( rc != SQLITE_OK )
 	{
-		logg("dbclose() - SQL error (%i): %s", rc, sqlite3_errmsg(FTL_db));
+		logg("Encountered error while trying to close database: %s", sqlite3_errstr(rc));
+		database = false;
 	}
 
-	// Set database pointer to NULL
-	FTL_db = NULL;
-
 	// Unlock mutex on the database
 	pthread_mutex_unlock(&dblock);
 }
 
 bool dbopen(void)
 {
+	// Lock mutex on the database
 	pthread_mutex_lock(&dblock);
+
+	// Try to open database
 	int rc = sqlite3_open_v2(FTLfiles.FTL_db, &FTL_db, SQLITE_OPEN_READWRITE, NULL);
-	if( rc != SQLITE_OK ){
-		logg("dbopen() - SQL error (%i): %s", rc, sqlite3_errmsg(FTL_db));
-		dbclose();
-		check_database(rc);
+	if( rc != SQLITE_OK )
+	{
+		logg("Encountered error while trying to open database: %s", sqlite3_errstr(rc));
+		database = false;
+		pthread_mutex_unlock(&dblock);
 		return false;
 	}
 
 	// Explicitly set busy handler to value defined in FTL.h
 	rc = sqlite3_busy_timeout(FTL_db, DATABASE_BUSY_TIMEOUT);
-	if(rc != SQLITE_OK)
+	if( rc != SQLITE_OK )
 	{
-		logg("dbopen() - Cannot set busy handler (%i): %s", rc, sqlite3_errmsg(FTL_db));
+		logg("Encountered error while trying to set busy timeout (%d ms) on database: %s",
+		     DATABASE_BUSY_TIMEOUT, sqlite3_errmsg(FTL_db));
+		database = false;
+		dbclose();
+		return false;
 	}
 
 	return true;
@@ -104,7 +90,7 @@ int dbquery(const char *format, ...)
 
 	// Log generated SQL string when dbquery() is called
 	// although the database connection is not available
-	if(!database)
+	if(database == false || ( FTL_db == NULL && !dbopen() ))
 	{
 		logg("dbquery(\"%s\") called but database is not available!", query);
 		sqlite3_free(query);
@@ -117,14 +103,21 @@ int dbquery(const char *format, ...)
 	}
 
 	int rc = sqlite3_exec(FTL_db, query, NULL, NULL, NULL);
-
 	if( rc != SQLITE_OK ){
-		logg("ERROR: SQL query failed with code %d: %s", rc, query);
-		check_database(rc);
+		logg("ERROR: SQL query \"%s\" failed: %s",
+		     query, sqlite3_errstr(rc));
+		dbclose();
 		return rc;
 	}
+
 	// Free allocated memory for query string
 	sqlite3_free(query);
+
+	if(config.debug & DEBUG_DATABASE)
+	{
+		logg("         ---> OK");
+	}
+
 	// Return success
 	return SQLITE_OK;
 }
@@ -156,9 +149,9 @@ static bool create_counter_table(void)
 static bool db_create(void)
 {
 	int rc = sqlite3_open_v2(FTLfiles.FTL_db, &FTL_db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, NULL);
-	if( rc != SQLITE_OK ){
-		logg("db_create() - SQL error (%i): %s", rc, sqlite3_errmsg(FTL_db));
-		check_database(rc);
+	if( rc != SQLITE_OK )
+	{
+		logg("Encountered error while trying to create database in rw-mode: %s", sqlite3_errstr(rc));
 		return false;
 	}
 	// Create Queries table in the database
@@ -211,16 +204,6 @@ void SQLite3LogCallback(void *pArg, int iErrCode, const char *zMsg)
 
 void db_init(void)
 {
-	// First check if the user doesn't want to use the database and set
-	// an empty string as file name in FTL's config file or configured
-	// a maximum history of zero days
-	if(FTLfiles.FTL_db == NULL || strlen(FTLfiles.FTL_db) == 0 ||
-	   config.maxDBdays == 0)
-	{
-		database = false;
-		return;
-	}
-
 	// Initialize database lock mutex
 	int rc;
 	if((rc = pthread_mutex_init(&dblock, NULL)) != 0)
@@ -230,6 +213,13 @@ void db_init(void)
 		exit(EXIT_FAILURE);
 	}
 
+	if(!use_database())
+	{
+		logg("Not using the long-term database");
+		database = false;
+		return;
+	}
+
 	// Lock database thread
 	pthread_mutex_lock(&dblock);
 
@@ -247,7 +237,6 @@ void db_init(void)
 		{
 			logg("Creation of database failed, database is not available");
 			pthread_mutex_unlock(&dblock);
-
 			database = false;
 			return;
 		}
@@ -256,27 +245,14 @@ void db_init(void)
 	// Try to open the database connection
 	rc = sqlite3_open_v2(FTLfiles.FTL_db, &FTL_db, SQLITE_OPEN_READWRITE, NULL);
 	if( rc != SQLITE_OK ){
-		logg("db_init() - Cannot open database (%i): %s", rc, sqlite3_errmsg(FTL_db));
-		dbclose();
-
+		logg("Cannot initialize (open) long-term database: %s", sqlite3_errstr(rc));
 		database = false;
 		return;
 	}
 
 	// Test FTL_db version and see if we need to upgrade the database file
 	int dbversion = db_get_FTL_property(DB_VERSION);
-	if(dbversion < 1)
-	{
-		logg("Database version incorrect (%i), database not available", dbversion);
-		dbclose();
-
-		database = false;
-		return;
-	}
-	else
-	{
-		logg("Database version is %i", dbversion);
-	}
+	logg("Database version is %i", dbversion);
 
 	// Update to version 2 if lower
 	if(dbversion < 2)
@@ -355,7 +331,7 @@ void db_init(void)
 
 int db_get_FTL_property(const unsigned int ID)
 {
-	if(!database)
+	if(!database || FTL_db == NULL)
 	{
 		logg("db_get_FTL_property(%u) called but database is not available!", ID);
 		return DB_FAILED;
@@ -378,7 +354,7 @@ int db_get_FTL_property(const unsigned int ID)
 
 bool db_set_FTL_property(const unsigned int ID, const int value)
 {
-	if(!database)
+	if(!database || FTL_db == NULL)
 	{
 		logg("db_set_FTL_property(%u, %i) called but database is not available!", ID, value);
 		return false;
@@ -388,7 +364,7 @@ bool db_set_FTL_property(const unsigned int ID, const int value)
 
 bool db_set_counter(const unsigned int ID, const int value)
 {
-	if(!database)
+	if(!database || FTL_db == NULL)
 	{
 		logg("db_set_counter(%u, %i) called but database is not available!", ID, value);
 		return false;
@@ -398,7 +374,7 @@ bool db_set_counter(const unsigned int ID, const int value)
 
 bool db_update_counters(const int total, const int blocked)
 {
-	if(!database)
+	if(!database || FTL_db == NULL)
 	{
 		logg("db_update_counters(%i, %i) called but database is not available!", total, blocked);
 		return false;
@@ -412,17 +388,22 @@ bool db_update_counters(const int total, const int blocked)
 
 int db_query_int(const char* querystr)
 {
-	if(!database)
+	if(!database || FTL_db == NULL)
 	{
 		logg("db_query_int(\"%s\") called but database is not available!", querystr);
 		return DB_FAILED;
 	}
 
+	if(config.debug & DEBUG_DATABASE)
+	{
+		logg("dbquery: \"%s\"", querystr);
+	}
+
 	sqlite3_stmt* stmt;
 	int rc = sqlite3_prepare_v2(FTL_db, querystr, -1, &stmt, NULL);
-	if( rc != SQLITE_OK ){
-		logg("db_query_int(%s) - SQL error prepare (%i): %s", querystr, rc, sqlite3_errmsg(FTL_db));
-		check_database(rc);
+	if( rc != SQLITE_OK )
+	{
+		logg("Encountered prepare error in db_query_int(\"%s\"): %s", querystr, sqlite3_errstr(rc));
 		return DB_FAILED;
 	}
 
@@ -432,61 +413,78 @@ int db_query_int(const char* querystr)
 	if( rc == SQLITE_ROW )
 	{
 		result = sqlite3_column_int(stmt, 0);
+
+		if(config.debug & DEBUG_DATABASE)
+		{
+			logg("         ---> Result %i (int)", result);
+		}
 	}
 	else if( rc == SQLITE_DONE )
 	{
 		// No rows available
 		result = DB_NODATA;
+
+		if(config.debug & DEBUG_DATABASE)
+		{
+			logg("         ---> No data");
+		}
 	}
 	else
 	{
-		logg("db_query_int(%s) - SQL error step (%i): %s", querystr, rc, sqlite3_errmsg(FTL_db));
-		check_database(rc);
+		logg("Encountered step error in db_query_int(\"%s\"): %s", querystr, sqlite3_errstr(rc));
 		return DB_FAILED;
 	}
 
 	sqlite3_finalize(stmt);
-
 	return result;
 }
 
 long int get_max_query_ID(void)
 {
-	if(!database)
+	if(!database || FTL_db == NULL)
 	{
 		logg("get_max_query_ID() called but database is not available!");
 		return DB_FAILED;
 	}
 
-	sqlite3_stmt* stmt;
+	const char *sql = "SELECT MAX(ID) FROM queries";
+	if(config.debug & DEBUG_DATABASE)
+	{
+		logg("dbquery: \"%s\"", sql);
+	}
 
-	int rc = sqlite3_prepare_v2(FTL_db, "SELECT MAX(ID) FROM queries", -1, &stmt, NULL);
-	if( rc != SQLITE_OK ){
-		logg("get_max_query_ID() - SQL error prepare (%i): %s", rc, sqlite3_errmsg(FTL_db));
+	sqlite3_stmt* stmt = NULL;
+	int rc = sqlite3_prepare_v2(FTL_db, sql, -1, &stmt, NULL);
+	if( rc != SQLITE_OK )
+	{
+		logg("Encountered prepare error in get_max_query_ID(): %s", sqlite3_errstr(rc));
+		database = false;
 		dbclose();
-		check_database(rc);
 		return DB_FAILED;
 	}
 
 	rc = sqlite3_step(stmt);
-	if( rc != SQLITE_ROW ){
-		logg("get_max_query_ID() - SQL error step (%i): %s", rc, sqlite3_errmsg(FTL_db));
+	if( rc != SQLITE_ROW )
+	{
+		logg("Encountered step error in get_max_query_ID(): %s", sqlite3_errstr(rc));
+		database = false;
 		dbclose();
-		check_database(rc);
 		return DB_FAILED;
 	}
 
 	sqlite3_int64 result = sqlite3_column_int64(stmt, 0);
-
+	if(config.debug & DEBUG_DATABASE)
+	{
+		logg("         ---> Result %lli (long long int)", (long long int)result);
+	}
 	sqlite3_finalize(stmt);
-
 	return result;
 }
 
 // Returns ID of the most recent successful INSERT.
 long get_lastID(void)
 {
-	if(!database)
+	if(!database || FTL_db == NULL)
 	{
 		logg("get_lastID() called but database is not available!");
 		return DB_FAILED;
@@ -499,3 +497,19 @@ const char *get_sqlite3_version(void)
 {
 	return sqlite3_libversion();
 }
+
+// Should the long-term database be used?
+__attribute__ ((pure)) bool use_database()
+{
+	// Check if the user doesn't want to use the database and set an
+	// empty string as file name in FTL's config file or configured
+	// a maximum history of zero days.
+	if(FTLfiles.FTL_db == NULL ||
+	   strlen(FTLfiles.FTL_db) == 0 ||
+	   config.maxDBdays == 0)
+	{
+		return false;
+	}
+
+	return true;
+}

src/database/common.h

@@ -12,7 +12,6 @@
 
 #include "sqlite3.h"
 
-bool check_database(int rc);
 void db_init(void);
 int db_get_FTL_property(const unsigned int ID);
 bool db_set_FTL_property(const unsigned int ID, const int value);
@@ -29,6 +28,7 @@ long int get_max_query_ID(void);
 bool db_set_counter(const unsigned int ID, const int value);
 bool db_update_counters(const int total, const int blocked);
 const char *get_sqlite3_version(void);
+bool use_database(void)  __attribute__ ((pure));
 
 extern sqlite3 *FTL_db;
 extern bool database;

src/database/database-thread.c

@@ -25,14 +25,18 @@
 
 void *DB_thread(void *val)
 {
-	int lastDBsave = 0;
-
 	// Set thread name
 	prctl(PR_SET_NAME,"database",0,0,0);
 
+	// First check if the user doesn't want to use the database.
+	// If this is the case, we terminate the database thread as
+	// there is no point in having it around
+	if(!use_database())
+		return NULL;
+
 	// Save timestamp as we do not want to store immediately
 	// to the database
-	lastDBsave = time(NULL) - time(NULL)%config.DBinterval;
+	time_t lastDBsave = time(NULL) - time(NULL)%config.DBinterval;
 
 	while(!killed && database)
 	{

src/database/network-table.c

@@ -137,21 +137,31 @@ void parse_neighbor_cache(void)
 	unsigned int entries = 0;
 	time_t now = time(NULL);
 
-	// Start collecting database commands
-	lock_shm();
+	const char *sql = "BEGIN TRANSACTION IMMEDIATE";
+	int rc = dbquery(sql);
+	if( rc != SQLITE_OK )
+	{
+		const char *text;
+		if( rc == SQLITE_BUSY )
+		{
+			text = "WARNING";
+		}
+		else
+		{
+			text = "ERROR";
+			// We shall not use the database any longer
+			database = false;
+		}
 
-	int ret = dbquery("BEGIN TRANSACTION");
-
-	if(ret == SQLITE_BUSY) {
-		logg("WARN: parse_neighbor_cache(), database is busy, skipping");
-		unlock_shm();
-		return;
-	} else if(ret != SQLITE_OK) {
-		logg("ERROR: parse_neighbor_cache() failed!");
-		unlock_shm();
+		// dbquery() above already logs the reson for why the query failed
+		logg("%s: Storing devices in network table (%s) failed", text, sql);
+		dbclose();
 		return;
 	}
 
+	// Start collecting database commands
+	lock_shm();
+
 	// Read ARP cache line by line
 	while(getline(&linebuffer, &linebuffersize, arpfp) != -1)
 	{
@@ -174,10 +184,10 @@ void parse_neighbor_cache(void)
 		// commitment. Read-only access such as this SELECT command will be
 		// executed immediately on the database.
 		char* querystr = NULL;
-		ret = asprintf(&querystr, "SELECT id FROM network WHERE hwaddr = \'%s\';", hwaddr);
-		if(querystr == NULL || ret < 0)
+		rc = asprintf(&querystr, "SELECT id FROM network WHERE hwaddr = \'%s\';", hwaddr);
+		if(querystr == NULL || rc < 0)
 		{
-			logg("Memory allocation failed in parse_arp_cache(): %i", ret);
+			logg("Memory allocation failed in parse_arp_cache(): %i", rc);
 			break;
 		}
 
@@ -214,16 +224,22 @@ void parse_neighbor_cache(void)
 		if(dbID == DB_NODATA)
 		{
 			char* macVendor = getMACVendor(hwaddr);
-			dbquery("INSERT INTO network "\
-			        "(hwaddr,interface,firstSeen,lastQuery,numQueries,name,macVendor) "\
-			        "VALUES (\'%s\',\'%s\',%lu, %ld, %u, \'%s\', \'%s\');",\
-			        hwaddr, iface, now,
-			        client != NULL ? client->lastQuery : 0L,
-			        client != NULL ? client->numQueriesARP : 0u,
-			        hostname,
-			        macVendor);
+			rc = dbquery("INSERT INTO network "\
+			             "(hwaddr,interface,firstSeen,lastQuery,numQueries,name,macVendor) "\
+			             "VALUES (\'%s\',\'%s\',%lu, %ld, %u, \'%s\', \'%s\');",\
+			             hwaddr, iface, now,
+			             client != NULL ? client->lastQuery : 0L,
+			             client != NULL ? client->numQueriesARP : 0u,
+			             hostname,
+			             macVendor);
 			free(macVendor);
 
+			if(rc != SQLITE_OK)
+			{
+				// SQLite error
+				break;
+			}
+
 			// Obtain ID which was given to this new entry
 			dbID = get_lastID();
 		}
@@ -234,27 +250,45 @@ void parse_neighbor_cache(void)
 			// client->lastQuery may be zero if this
 			// client is only known from a database entry but has
 			// not been seen since then
-			dbquery("UPDATE network "\
-			        "SET lastQuery = MAX(lastQuery, %ld) "\
-			        "WHERE id = %i;",\
-			        client->lastQuery, dbID);
+			rc = dbquery("UPDATE network "\
+			             "SET lastQuery = MAX(lastQuery, %ld) "\
+			             "WHERE id = %i;",\
+			             client->lastQuery, dbID);
+
+			if(rc != SQLITE_OK)
+			{
+				// SQLite error
+				break;
+			}
 
 			// Update numQueries. Add queries seen since last update
 			// and reset counter afterwards
-			dbquery("UPDATE network "\
-			        "SET numQueries = numQueries + %u "\
-			        "WHERE id = %i;",\
-			        client->numQueriesARP, dbID);
+			rc = dbquery("UPDATE network "\
+			             "SET numQueries = numQueries + %u "\
+			             "WHERE id = %i;",\
+			             client->numQueriesARP, dbID);
 			client->numQueriesARP = 0;
 
+			if(rc != SQLITE_OK)
+			{
+				// SQLite error
+				break;
+			}
+
 			// Store hostname if available
 			if(strlen(hostname) > 0)
 			{
 				// Store host name
-				dbquery("UPDATE network "\
-				        "SET name = \'%s\' "\
-				        "WHERE id = %i;",\
-				        hostname, dbID);
+				rc = dbquery("UPDATE network "\
+				             "SET name = \'%s\' "\
+				             "WHERE id = %i;",\
+				             hostname, dbID);
+
+				if(rc != SQLITE_OK)
+				{
+					// SQLite error
+					break;
+				}
 			}
 		}
 		// else:
@@ -265,17 +299,59 @@ void parse_neighbor_cache(void)
 		// this pair already exists, the UNIQUE(network_id,ip) trigger
 		// becomes active and the line is instead REPLACEd, causing the
 		// lastQuery timestamp to be updated
-		dbquery("INSERT OR REPLACE INTO network_addresses "\
-		        "(network_id,ip) VALUES(%i,\'%s\');", dbID, ip);
+		rc = dbquery("INSERT OR REPLACE INTO network_addresses "\
+		             "(network_id,ip) VALUES(%i,\'%s\');", dbID, ip);
+
+		if(rc != SQLITE_OK)
+		{
+			// SQLite error
+			break;
+		}
 
 		// Count number of processed ARP cache entries
 		entries++;
 	}
 
-	// Actually update the database
-	if(dbquery("COMMIT") != SQLITE_OK) {
-		logg("ERROR: parse_neighbor_cache() failed!");
+	// Check for possible error in loop
+	if(rc != SQLITE_OK)
+	{
+		const char *text;
+		if( rc == SQLITE_BUSY )
+		{
+			text = "WARNING";
+		}
+		else
+		{
+			text = "ERROR";
+			// We shall not use the database any longer
+			database = false;
+		}
+
+		// dbquery() above already logs the reson for why the query failed
+		logg("%s: Storing devices in network table failed", text);
 		unlock_shm();
+		dbclose();
+		return;
+	}
+
+	// Actually update the database
+	if((rc = dbquery("END TRANSACTION")) != SQLITE_OK) {
+		const char *text;
+		if( rc == SQLITE_BUSY )
+		{
+			text = "WARNING";
+		}
+		else
+		{
+			text = "ERROR";
+			// We shall not use the database any longer
+			database = false;
+		}
+
+		// dbquery() above already logs the reson for why the query failed
+		logg("%s: Storing devices in network table failed", text);
+		unlock_shm();
+		dbclose();
 		return;
 	}
 
@@ -308,20 +384,19 @@ bool unify_hwaddr(void)
 
 	// Perform SQL query
 	sqlite3_stmt* stmt;
-	int ret = sqlite3_prepare_v2(FTL_db, querystr, -1, &stmt, NULL);
-	if( ret != SQLITE_OK ){
-		logg("unify_hwaddr(%s) - SQL error prepare (%i): %s", querystr, ret, sqlite3_errmsg(FTL_db));
-		check_database(ret);
+	int rc = sqlite3_prepare_v2(FTL_db, querystr, -1, &stmt, NULL);
+	if( rc != SQLITE_OK ){
+		logg("unify_hwaddr(\"%s\") - SQL error prepare: %s", querystr, sqlite3_errstr(rc));
 		return false;
 	}
 
 	// Loop until no further (id,hwaddr) sets are available
-	while((ret = sqlite3_step(stmt)) != SQLITE_DONE)
+	while((rc = sqlite3_step(stmt)) != SQLITE_DONE)
 	{
 		// Check if we ran into an error
-		if(ret != SQLITE_ROW)
+		if(rc != SQLITE_ROW)
 		{
-			logg("unify_hwaddr(%s) - SQL error step (%i): %s", querystr, ret, sqlite3_errmsg(FTL_db));
+			logg("unify_hwaddr(\"%s\") - SQL error step: %s", querystr, sqlite3_errstr(rc));
 			dbclose();
 			return false;
 		}
@@ -371,21 +446,21 @@ static char* getMACVendor(const char* hwaddr)
 	{
 		// File does not exist
 		if(config.debug & DEBUG_ARP)
-			logg("getMACVenor(%s): %s does not exist", hwaddr, FTLfiles.macvendor_db);
+			logg("getMACVenor(\"%s\"): %s does not exist", hwaddr, FTLfiles.macvendor_db);
 		return strdup("");
 	}
 	else if(strlen(hwaddr) != 17)
 	{
 		// MAC address is incomplete
 		if(config.debug & DEBUG_ARP)
-			logg("getMACVenor(%s): MAC invalid (length %zu)", hwaddr, strlen(hwaddr));
+			logg("getMACVenor(\"%s\"): MAC invalid (length %zu)", hwaddr, strlen(hwaddr));
 		return strdup("");
 	}
 
 	sqlite3 *macvendor_db = NULL;
 	int rc = sqlite3_open_v2(FTLfiles.macvendor_db, &macvendor_db, SQLITE_OPEN_READONLY, NULL);
 	if( rc != SQLITE_OK ){
-		logg("getMACVendor(%s) - SQL error (%i): %s", hwaddr, rc, sqlite3_errmsg(macvendor_db));
+		logg("getMACVendor(\"%s\") - SQL error: %s", hwaddr, sqlite3_errstr(rc));
 		sqlite3_close(macvendor_db);
 		return strdup("");
 	}
@@ -397,7 +472,7 @@ static char* getMACVendor(const char* hwaddr)
 	rc = asprintf(&querystr, "SELECT vendor FROM macvendor WHERE mac LIKE \'%s\';", hwaddrshort);
 	if(rc < 1)
 	{
-		logg("getMACVendor(%s) - Allocation error (%i)", hwaddr, rc);
+		logg("getMACVendor(\"%s\") - Allocation error (%i)", hwaddr, rc);
 		sqlite3_close(macvendor_db);
 		return strdup("");
 	}
@@ -406,7 +481,7 @@ static char* getMACVendor(const char* hwaddr)
 	sqlite3_stmt* stmt = NULL;
 	rc = sqlite3_prepare_v2(macvendor_db, querystr, -1, &stmt, NULL);
 	if( rc != SQLITE_OK ){
-		logg("getMACVendor(%s) - SQL error prepare (%s, %i): %s", hwaddr, querystr, rc, sqlite3_errmsg(macvendor_db));
+		logg("getMACVendor(\"%s\") - SQL error prepare \"%s\": %s", hwaddr, querystr, sqlite3_errstr(rc));
 		sqlite3_close(macvendor_db);
 		return strdup("");
 	}
@@ -427,7 +502,7 @@ static char* getMACVendor(const char* hwaddr)
 	if(rc != SQLITE_DONE && rc != SQLITE_ROW)
 	{
 		// Error
-		logg("getMACVendor(%s) - SQL error step (%i): %s", hwaddr, rc, sqlite3_errmsg(macvendor_db));
+		logg("getMACVendor(\"%s\") - SQL error step: %s", hwaddr, sqlite3_errstr(rc));
 	}
 
 	sqlite3_finalize(stmt);
@@ -454,7 +529,7 @@ void updateMACVendorRecords(void)
 	const char* selectstr = "SELECT id,hwaddr FROM network;";
 	int rc = sqlite3_prepare_v2(FTL_db, selectstr, -1, &stmt, NULL);
 	if( rc != SQLITE_OK ){
-		logg("updateMACVendorRecords() - SQL error prepare (%s, %i): %s", selectstr, rc, sqlite3_errmsg(FTL_db));
+		logg("updateMACVendorRecords() - SQL error prepare \"%s\": %s", selectstr, sqlite3_errstr(rc));
 		dbclose();
 		return;
 	}
@@ -473,7 +548,7 @@ void updateMACVendorRecords(void)
 		char *updatestr = NULL;
 		if(asprintf(&updatestr, "UPDATE network SET macVendor = \'%s\' WHERE id = %i", vendor, id) < 1)
 		{
-			logg("updateMACVendorRecords() - Allocation error 2");
+			logg("updateMACVendorRecords() - Allocation error");
 			free(vendor);
 			break;
 		}
@@ -482,7 +557,7 @@ void updateMACVendorRecords(void)
 		char *zErrMsg = NULL;
 		rc = sqlite3_exec(FTL_db, updatestr, NULL, NULL, &zErrMsg);
 		if( rc != SQLITE_OK ){
-			logg("updateMACVendorRecords() - SQL exec error: %s (%i): %s", updatestr, rc, zErrMsg);
+			logg("updateMACVendorRecords() - SQL exec error: \"%s\": %s", updatestr, zErrMsg);
 			sqlite3_free(zErrMsg);
 			free(updatestr);
 			free(vendor);
@@ -496,7 +571,7 @@ void updateMACVendorRecords(void)
 	if(rc != SQLITE_DONE)
 	{
 		// Error
-		logg("updateMACVendorRecords() - SQL error step (%i): %s", rc, sqlite3_errmsg(FTL_db));
+		logg("updateMACVendorRecords() - SQL error step: %s", sqlite3_errstr(rc));
 	}
 
 	sqlite3_finalize(stmt);
@@ -508,7 +583,7 @@ char* __attribute__((malloc)) getDatabaseHostname(const char* ipaddr)
 	// Open pihole-FTL.db database file
 	if(!dbopen())
 	{
-		logg("getDatabaseHostname(%s) - Failed to open DB", ipaddr);
+		logg("getDatabaseHostname(\"%s\") - Failed to open DB", ipaddr);
 		return strdup("");
 	}
 
@@ -517,16 +592,16 @@ char* __attribute__((malloc)) getDatabaseHostname(const char* ipaddr)
 	const char *querystr = "SELECT name FROM network WHERE id = (SELECT network_id FROM network_addresses WHERE ip = ?);";
 	int rc = sqlite3_prepare_v2(FTL_db, querystr, -1, &stmt, NULL);
 	if( rc != SQLITE_OK ){
-		logg("getDatabaseHostname(%s) - SQL error prepare (%i): %s",
-		     ipaddr, rc, sqlite3_errmsg(FTL_db));
+		logg("getDatabaseHostname(\"%s\") - SQL error prepare: %s",
+		     ipaddr, sqlite3_errstr(rc));
 		return strdup("");
 	}
 
 	// Bind ipaddr to prepared statement
 	if((rc = sqlite3_bind_text(stmt, 1, ipaddr, -1, SQLITE_STATIC)) != SQLITE_OK)
 	{
-		logg("getDatabaseHostname(%s): Failed to bind domain (error %d) - %s",
-		     ipaddr, rc, sqlite3_errmsg(FTL_db));
+		logg("getDatabaseHostname(\"%s\"): Failed to bind domain: %s",
+		     ipaddr, sqlite3_errstr(rc));
 		sqlite3_reset(stmt);
 		sqlite3_finalize(stmt);
 		return strdup("");

src/database/query-table.c

@@ -30,7 +30,6 @@ int get_number_of_queries_in_DB(void)
 	// We need to handle opening/closing of the database herein.
 	if(!dbopen())
 	{
-		logg("number_of_queries_in_DB() - Failed to open database.");
 		return DB_FAILED;
 	}
 
@@ -56,21 +55,36 @@ void DB_save_queries(void)
 	// Open database
 	if(!dbopen())
 	{
-		logg("DB_save_queries() - failed to open FTL_db");
+		logg("Failed to open long-term database when trying to store queries");
 		return;
 	}
 
-	unsigned int saved = 0, saved_error = 0;
+	unsigned int saved = 0;
+	bool error = false;
 	sqlite3_stmt* stmt = NULL;
 
 	// Get last ID stored in the database
 	long int lastID = get_max_query_ID();
 
-	int rc;
-	if((rc = dbquery("BEGIN TRANSACTION IMMEDIATE")) != SQLITE_OK)
+	const char *sql = "BEGIN TRANSACTION IMMEDIATE;";
+	int rc = dbquery(sql);
+	if( rc != SQLITE_OK )
 	{
-		logg("BEGIN TRANSACTION IMMEDIATE failed when trying to store queries to long-term database: %s.",
-		     sqlite3_errstr(rc));
+		const char *text;
+		if( rc == SQLITE_BUSY )
+		{
+			text = "WARNING";
+		}
+		else
+		{
+			text = "ERROR";
+			// We shall not use the database any longer
+			database = false;
+		}
+
+		// dbquery() above already logs the reson for why the query failed
+		logg("%s: Storing queries in long-term database (%s) failed", text, sql);
+		dbclose();
 		return;
 	}
 
@@ -78,7 +92,8 @@ void DB_save_queries(void)
 	if( rc != SQLITE_OK )
 	{
 		logg("DB_save_queries() - error in preparing SQL statement (%i): %s", rc, sqlite3_errmsg(FTL_db));
-		check_database(rc);
+		database = false;
+		dbclose();
 		return;
 	}
 
@@ -143,20 +158,11 @@ void DB_save_queries(void)
 		sqlite3_clear_bindings(stmt);
 		sqlite3_reset(stmt);
 
-		if( rc != SQLITE_DONE ){
-			logg("DB_save_queries() - SQL error (%i): %s", rc, sqlite3_errmsg(FTL_db));
-			saved_error++;
-			if(saved_error < 3)
-			{
-				continue;
-			}
-			else
-			{
-				logg("DB_save_queries() - exiting due to too many errors");
-				break;
-			}
-			// Check this error message
-			check_database(rc);
+		if( rc != SQLITE_DONE )
+		{
+			logg("Encountered error while trying to store queries in long-term database: %s", sqlite3_errstr(rc));
+			error = true;
+			break;
 		}
 
 		saved++;
@@ -181,22 +187,34 @@ void DB_save_queries(void)
 			newlasttimestamp = query->timestamp;
 	}
 
+	if((rc = sqlite3_finalize(stmt)) != SQLITE_OK)
+	{
+		logg("Statement finalization failed when trying to store queries to long-term database: %s.",
+		     sqlite3_errstr(rc));
+
+		if( rc != SQLITE_BUSY )
+			database = false;
+
+		dbclose();
+		return;
+	}
+
 	// Finish prepared statement
 	if((rc = dbquery("END TRANSACTION")) != SQLITE_OK)
 	{
 		logg("END TRANSACTION failed when trying to store queries to long-term database: %s.",
 		     sqlite3_errstr(rc));
-		saved_error++;
-	}
-	if((rc = sqlite3_finalize(stmt)) != SQLITE_OK)
-	{
-		check_database(rc);
+
+		if( rc != SQLITE_BUSY )
+			database = false;
+
+		dbclose();
 		return;
 	}
 
 	// Store index for next loop interation round and update last time stamp
 	// in the database only if all queries have been saved successfully
-	if(saved > 0 && saved_error == 0)
+	if(saved > 0 && !error)
 	{
 		lastdbindex = queryID;
 		db_set_FTL_property(DB_LASTTIMESTAMP, newlasttimestamp);
@@ -209,7 +227,7 @@ void DB_save_queries(void)
 	if(config.debug & DEBUG_DATABASE)
 	{
 		logg("Notice: Queries stored in FTL_db: %u (took %.1f ms, last SQLite ID %li)", saved, timer_elapsed_msec(DATABASE_WRITE_TIMER), lastID);
-		if(saved_error > 0)
+		if(error)
 			logg("        There are queries that have not been saved");
 	}
 }
@@ -219,7 +237,7 @@ void delete_old_queries_in_DB(void)
 	// Open database
 	if(!dbopen())
 	{
-		logg("Failed to open FTL_db in delete_old_queries_in_DB()");
+		logg("Failed to open long-term database when trying to delete old queries");
 		return;
 	}
 
@@ -252,7 +270,7 @@ void DB_read_queries(void)
 	// Open database file
 	if(!dbopen())
 	{
-		logg("DB_read_queries() - Failed to open FTL_db");
+		logg("Failed to open long-term database when trying to read queries");
 		return;
 	}
 
@@ -262,9 +280,10 @@ void DB_read_queries(void)
 	const time_t mintime = now - config.maxlogage;
 	char *querystr = NULL;
 	int rc = asprintf(&querystr, "SELECT * FROM queries WHERE timestamp >= %li", mintime);
-	if(rc < 1)
+	if(rc < 42)
 	{
-		logg("DB_read_queries() - Allocation error (%i): %s", rc, sqlite3_errmsg(FTL_db));
+		logg("DB_read_queries() - Memory allocation error: %s", sqlite3_errstr(rc));
+		dbclose();
 		return;
 	}
 	// Log FTL_db query string in debug mode
@@ -275,8 +294,8 @@ void DB_read_queries(void)
 	sqlite3_stmt* stmt = NULL;
 	rc = sqlite3_prepare_v2(FTL_db, querystr, -1, &stmt, NULL);
 	if( rc != SQLITE_OK ){
-		logg("DB_read_queries() - SQL error prepare (%i): %s", rc, sqlite3_errmsg(FTL_db));
-		check_database(rc);
+		logg("DB_read_queries() - SQL error prepare: %s", sqlite3_errstr(rc));
+		dbclose();
 		return;
 	}
 
@@ -454,7 +473,6 @@ void DB_read_queries(void)
 	if( rc != SQLITE_DONE ){
 		logg("DB_read_queries() - SQL error step (%i): %s", rc, sqlite3_errmsg(FTL_db));
 		dbclose();
-		check_database(rc);
 		return;
 	}