From 028989bcae09040a14d1fdaa21ab48e2c1b2ba83 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Nov 2021 13:01:39 +0100 Subject: [PATCH] Reduce code duplication by using labels + ensure we do not run dbclose() multiple times on database issues (e.g., when database is locked) Signed-off-by: DL6ER --- src/daemon.c | 2 -- src/database/aliasclients.c | 7 ++-- src/database/common.c | 6 ---- src/database/message-table.c | 34 ++++++++---------- src/database/network-table.c | 67 ++++++++++++++++++++---------------- src/database/query-table.c | 33 ++++++++---------- 6 files changed, 68 insertions(+), 81 deletions(-) diff --git a/src/daemon.c b/src/daemon.c index 2a2e3256..1c2c77ee 100644 --- a/src/daemon.c +++ b/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() diff --git a/src/database/aliasclients.c b/src/database/aliasclients.c index 30cba9c6..cf990d63 100644 --- a/src/database/aliasclients.c +++ b/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); } \ No newline at end of file diff --git a/src/database/common.c b/src/database/common.c index e7c075ca..586220b6 100644 --- a/src/database/common.c +++ b/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; } diff --git a/src/database/message-table.c b/src/database/message-table.c index 5badc17e..42bfa4c2 100644 --- a/src/database/message-table.c +++ b/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) diff --git a/src/database/network-table.c b/src/database/network-table.c index db39f4d6..7363a33e 100644 --- a/src/database/network-table.c +++ b/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; } diff --git a/src/database/query-table.c b/src/database/query-table.c index 574337f5..3a0fc2f9 100644 --- a/src/database/query-table.c +++ b/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); }