Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com --- inc/abrtlib.h | 3 + inc/database.h | 59 +++++++++++++++------ lib/plugins/Makefile.am | 4 +- lib/plugins/SQLite3.cpp | 122 ++++++++++++++++++++++++------------------ lib/plugins/SQLite3.h | 5 +- src/daemon/CrashWatcher.cpp | 24 ++++++--- src/daemon/MiddleWare.cpp | 94 +++++++++++++++++++++------------ 7 files changed, 198 insertions(+), 113 deletions(-)
diff --git a/inc/abrtlib.h b/inc/abrtlib.h index 2fc5101..a1ad9c3 100644 --- a/inc/abrtlib.h +++ b/inc/abrtlib.h @@ -64,6 +64,9 @@ int vdprintf(int d, const char *format, va_list ap); #undef NORETURN #define NORETURN __attribute__ ((noreturn))
+#undef ERR_PTR +#define ERR_PTR ((void*)(uintptr_t)1) + #undef ARRAY_SIZE #define ARRAY_SIZE(x) ((unsigned)(sizeof(x) / sizeof((x)[0])))
diff --git a/inc/database.h b/inc/database.h index df109bd..c197ddf 100644 --- a/inc/database.h +++ b/inc/database.h @@ -21,6 +21,7 @@ #ifndef DATABASE_H_ #define DATABASE_H_
+#include <glib.h> #include <string> #include <vector> #include "plugin.h" @@ -36,22 +37,48 @@ /** * A struct contains one database row. */ -typedef struct database_row_t +struct db_row { - std::string m_sUUID; /**< A local UUID.*/ - std::string m_sUID; /**< An UID of an user.*/ - std::string m_sInformAll; - std::string m_sDebugDumpDir; /**< A debugdump directory of a crash.*/ - std::string m_sCount; /**< Crash rate.*/ - std::string m_sReported; /**< Is a row reported?*/ - std::string m_sMessage; /**< if a row is reported, then there can be store message abotu that*/ - std::string m_sTime; /**< Time of last occurred crash with same local UUID*/ -} database_row_t; + char *db_uuid; /**< A local UUID.*/ + char *db_uid; /**< An UID of an user.*/ + char *db_inform_all; + char *db_dump_dir; /**< A debugdump directory of a crash.*/ + char *db_count; /**< Crash rate.*/ + char *db_reported; /**< Is a row reported?*/ + char *db_message; /**< if a row is reported, then there can be store message abotu that*/ + char *db_time; /**< Time of last occurred crash with same local UUID*/ +};
-/** - * A vector contains one or more database rows. - */ -typedef std::vector<database_row_t> vector_database_rows_t; +static inline void db_row_free(struct db_row *row) +{ + if (!row) + return; + + free(row->db_uuid); + free(row->db_uid); + free(row->db_inform_all); + free(row->db_dump_dir); + free(row->db_count); + free(row->db_reported); + free(row->db_message); + free(row->db_time); + + free(row); +} + +static inline void db_list_free(GList *list) +{ + if (!list) + return; + + struct db_row *row = NULL; + for (GList *li = list; li != NULL; li = g_list_next(li)) + { + row = (struct db_row*)li->data; + db_row_free(row); + } + g_list_free(list); +}
/** * An abstract class. The class defines a database plugin interface. @@ -104,7 +131,7 @@ class CDatabase : public CPlugin * @param pUID An UID of an user. * @return A vector of matched rows. */ - virtual vector_database_rows_t GetUIDData(long caller_uid) = 0; + virtual GList *GetUIDData(long caller_uid) = 0; /** * A method, which returns one row accordind to UUID of a crash and * UID of an user. If there are no row, empty row is returned. @@ -112,7 +139,7 @@ class CDatabase : public CPlugin * @param pUID An UID of an user. * @return A matched row. */ - virtual database_row_t GetRow(const char *crash_id) = 0; + virtual struct db_row *GetRow(const char *crash_id) = 0; };
#endif diff --git a/lib/plugins/Makefile.am b/lib/plugins/Makefile.am index a896e14..ca874e1 100644 --- a/lib/plugins/Makefile.am +++ b/lib/plugins/Makefile.am @@ -98,8 +98,8 @@ libMailx_la_CPPFLAGS = -I$(INC_PATH) -I$(UTILS_PATH) -DPLUGINS_LIB_DIR="$(PLUGI # SQLite3 libSQLite3_la_SOURCES = SQLite3.cpp SQLite3.h libSQLite3_la_LDFLAGS = -avoid-version -libSQLite3_la_LIBADD = $(SQLITE3_LIBS) -libSQLite3_la_CPPFLAGS = -I$(INC_PATH) -I$(UTILS_PATH) $(SQLITE3_CFLAGS) -DLOCALSTATEDIR='"$(localstatedir)"' +libSQLite3_la_LIBADD = $(SQLITE3_LIBS) $(GLIB_LIBS) +libSQLite3_la_CPPFLAGS = -I$(INC_PATH) -I$(UTILS_PATH) $(SQLITE3_CFLAGS) -DLOCALSTATEDIR='"$(localstatedir)"' $(GLIB_CFLAGS)
# Logger libLogger_la_SOURCES = Logger.cpp Logger.h diff --git a/lib/plugins/SQLite3.cpp b/lib/plugins/SQLite3.cpp index 322f15a..3cd9f7d 100644 --- a/lib/plugins/SQLite3.cpp +++ b/lib/plugins/SQLite3.cpp @@ -103,13 +103,9 @@ static string sql_escape(const char *str) /* Note: * expects "SELECT * FROM ...", not "SELECT <only some fields> FROM ..." */ -static void get_table(vector_database_rows_t& pTable, - sqlite3 *db, const char *fmt, ...) +static GList *vget_table(sqlite3 *db, const char *fmt, va_list p) { - va_list p; - va_start(p, fmt); char *sql = xvasprintf(fmt, p); - va_end(p);
char **table; int ncol, nrow; @@ -117,39 +113,54 @@ static void get_table(vector_database_rows_t& pTable, int ret = sqlite3_get_table(db, sql, &table, &nrow, &ncol, &err); if (ret != SQLITE_OK) { - string errstr = ssprintf("Error in SQL:'%s' error: %s", sql, err); + error_msg("Error in SQL:'%s' error: %s", sql, err); free(sql); sqlite3_free(err); - throw CABRTException(EXCEP_PLUGIN, errstr.c_str()); + return (GList*)ERR_PTR; } - VERB2 log("%d rows returned by SQL:%s", nrow, sql); + VERB2 log("%s: %d rows returned by SQL:%s", __func__, nrow, sql); free(sql);
- pTable.clear(); + GList *rows = NULL; int ii; for (ii = 0; ii < nrow; ii++) { int jj; - database_row_t row; + struct db_row *row = (struct db_row*)xzalloc(sizeof(struct db_row)); for (jj = 0; jj < ncol; jj++) { char *val = table[jj + (ncol*ii) + ncol]; switch (jj) { - case 0: row.m_sUUID = val; break; - case 1: row.m_sUID = val; break; - case 2: row.m_sInformAll = val; break; - case 3: row.m_sDebugDumpDir = val; break; - case 4: row.m_sCount = val; break; - case 5: row.m_sReported = val; break; - case 6: row.m_sTime = val; break; - case 7: row.m_sMessage = val; break; + case 0: row->db_uuid = xstrdup(val); break; + case 1: row->db_uid = xstrdup(val); break; + case 2: row->db_inform_all = xstrdup(val); break; + case 3: row->db_dump_dir = xstrdup(val); break; + case 4: row->db_count = xstrdup(val); break; + case 5: row->db_reported = xstrdup(val); break; + case 6: row->db_time = xstrdup(val); break; + case 7: row->db_message = xstrdup(val); break; } } - pTable.push_back(row); + rows = g_list_append(rows, row);
} sqlite3_free_table(table); + + return rows; +} + +static GList *get_table_or_die(sqlite3 *db, const char *fmt, ...) +{ + va_list p; + va_start(p, fmt); + GList *table = vget_table(db, fmt, p); + va_end(p); + + if (table == (GList*)ERR_PTR) + xfunc_die(); + + return table; }
static int execute_sql(sqlite3 *db, const char *fmt, ...) @@ -177,13 +188,17 @@ static int execute_sql(sqlite3 *db, const char *fmt, ...)
static bool exists_uuid_uid(sqlite3 *db, const char *UUID, const char *UID) { - vector_database_rows_t table; - get_table(table, db, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", - UUID, UID + GList *table = get_table_or_die(db, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", + UUID, UID ); - return !table.empty(); + + if (!table) + return false; + + db_list_free(table); + + return true; }
static void update_from_old_ver(sqlite3 *db, int old_version) @@ -496,27 +511,26 @@ void CSQLite3::DeleteRows_by_dir(const char *dump_dir) }
/* Get UID:UUID pair(s) to delete */ - vector_database_rows_t table; - get_table(table, m_pDB, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_DEBUG_DUMP_PATH"='%s';", - dump_dir + GList *table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_DEBUG_DUMP_PATH"='%s';", + dump_dir ); - if (table.empty()) + + if (!table) { return; }
+ struct db_row *row = NULL; /* Delete from both tables */ - vector_database_rows_t::iterator it = table.begin(); - while (it != table.end()) + for (GList *li = table; li != NULL; li = g_list_next(li)) { + row = (struct db_row*)li->data; execute_sql(m_pDB, "DELETE FROM "ABRT_REPRESULT_TABLE " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", - it->m_sUUID.c_str(), it->m_sUID.c_str() + row->db_uuid, row->db_uid ); - it++; } execute_sql(m_pDB, "DELETE FROM "ABRT_TABLE @@ -601,32 +615,31 @@ void CSQLite3::SetReportedPerReporter(const char *crash_id, } }
-vector_database_rows_t CSQLite3::GetUIDData(long caller_uid) +GList *CSQLite3::GetUIDData(long caller_uid) { - vector_database_rows_t table; + GList *table = NULL;
if (caller_uid == 0) { - get_table(table, m_pDB, "SELECT * FROM "ABRT_TABLE";"); + table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE";"); } else { - get_table(table, m_pDB, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_UID"='%ld' OR "COL_INFORMALL"=1;", - caller_uid + table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_UID"='%ld' OR "COL_INFORMALL"=1;", + caller_uid ); } return table; }
-database_row_t CSQLite3::GetRow(const char *crash_id) +struct db_row *CSQLite3::GetRow(const char *crash_id) { const char *UUID = strchr(crash_id, ':'); if (!UUID || !is_string_safe(crash_id) ) { - return database_row_t(); + return NULL; }
/* Split crash_id into UID:UUID */ @@ -636,18 +649,23 @@ database_row_t CSQLite3::GetRow(const char *crash_id) strncpy(UID, crash_id, uid_len); UID[uid_len] = '\0';
- vector_database_rows_t table; - get_table(table, m_pDB, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", - UUID, UID + GList *table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", + UUID, UID );
- if (table.size() == 0) + if (!table) { - return database_row_t(); + return NULL; } - return table[0]; + + GList *first = g_list_first(table); + struct db_row *row = (struct db_row*)xzalloc(sizeof(struct db_row)); + memcpy(row, first->data, sizeof(struct db_row)); + + db_list_free(table); + + return row; }
void CSQLite3::SetSettings(const map_plugin_settings_t& pSettings) diff --git a/lib/plugins/SQLite3.h b/lib/plugins/SQLite3.h index d2af864..3dfd0a9 100644 --- a/lib/plugins/SQLite3.h +++ b/lib/plugins/SQLite3.h @@ -47,10 +47,11 @@ class CSQLite3 : public CDatabase virtual void SetReportedPerReporter(const char *crash_id, const char *reporter, const char *pMessage); - virtual vector_database_rows_t GetUIDData(long caller_uid); - virtual database_row_t GetRow(const char *crash_id); + virtual GList *GetUIDData(long caller_uid); + virtual struct db_row *GetRow(const char *crash_id);
virtual void SetSettings(const map_plugin_settings_t& pSettings); };
+ #endif diff --git a/src/daemon/CrashWatcher.cpp b/src/daemon/CrashWatcher.cpp index 6310118..6e04628 100644 --- a/src/daemon/CrashWatcher.cpp +++ b/src/daemon/CrashWatcher.cpp @@ -188,27 +188,37 @@ int DeleteDebugDump(const char *crash_id, long caller_uid) { CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - database_row_t row = database->GetRow(crash_id); - if (row.m_sUUID == "") + struct db_row *row = database->GetRow(crash_id); + if (!row) + return EIO; + + if (!row->db_uuid) { database->DisConnect(); + db_row_free(row); return ENOENT; } + + char caller_uid_str[sizeof(long) * 3 + 2]; + sprintf(caller_uid_str, "%li", caller_uid); + if (caller_uid != 0 /* not called by root */ - && row.m_sInformAll != "1" - && to_string(caller_uid) != row.m_sUID + && row->db_inform_all[0] != '1' + && strcmp(caller_uid_str, row->db_uid) != 0 ) { database->DisConnect(); + db_row_free(row); return EPERM; } database->DeleteRow(crash_id); database->DisConnect(); - const char *dump_dir = row.m_sDebugDumpDir.c_str(); - if (dump_dir[0] != '\0') + if (row->db_dump_dir[0] != '\0') { - delete_debug_dump_dir(dump_dir); + delete_debug_dump_dir(row->db_dump_dir); + db_row_free(row); return 0; /* success */ } + db_row_free(row); } catch (CABRTException& e) { diff --git a/src/daemon/MiddleWare.cpp b/src/daemon/MiddleWare.cpp index 7752dc3..d97e10f 100644 --- a/src/daemon/MiddleWare.cpp +++ b/src/daemon/MiddleWare.cpp @@ -278,21 +278,27 @@ mw_result_t CreateCrashReport(const char *crash_id, { VERB2 log("CreateCrashReport('%s',%ld,result)", crash_id, caller_uid);
- database_row_t row; CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - row = database->GetRow(crash_id); + + struct db_row *row = database->GetRow(crash_id); database->DisConnect(); - if (row.m_sUUID == "") + if (row && !row->db_uuid) { error_msg("crash '%s' is not in database", crash_id); + db_row_free(row); return MW_IN_DB_ERROR; } + + char caller_uid_str[sizeof(long) * 3 + 2]; + sprintf(caller_uid_str, "%li", caller_uid); + if (caller_uid != 0 /* not called by root */ - && row.m_sInformAll != "1" - && to_string(caller_uid) != row.m_sUID + && row->db_inform_all[0] != '1' + && strcmp(caller_uid_str, row->db_uid) != 0 ) { error_msg("crash '%s' can't be accessed by user with uid %ld", crash_id, caller_uid); + db_row_free(row); return MW_IN_DB_ERROR; }
@@ -300,9 +306,10 @@ mw_result_t CreateCrashReport(const char *crash_id, try { dump_dir_t *dd = dd_init(); - if (!dd_opendir(dd, row.m_sDebugDumpDir.c_str())) + if (!dd_opendir(dd, row->db_dump_dir)) { dd_close(dd); + db_row_free(row); return MW_ERROR; }
@@ -319,24 +326,26 @@ mw_result_t CreateCrashReport(const char *crash_id, // Why do we reload dump dir's data via DebugDumpToCrashReport?
VERB3 log(" run_analyser_CreateReport('%s')", analyzer.c_str()); - run_analyser_CreateReport(analyzer.c_str(), row.m_sDebugDumpDir.c_str(), force); + run_analyser_CreateReport(analyzer.c_str(), row->db_dump_dir, force);
- std::string dup_hash = GetGlobalUUID(analyzer.c_str(), row.m_sDebugDumpDir.c_str()); + std::string dup_hash = GetGlobalUUID(analyzer.c_str(), row->db_dump_dir); VERB3 log(" DUPHASH:'%s'", dup_hash.c_str());
- VERB3 log(" RunAnalyzerActions('%s','%s','%s',force=%d)", analyzer.c_str(), package_name, row.m_sDebugDumpDir.c_str(), force); - RunAnalyzerActions(analyzer.c_str(), package_name, row.m_sDebugDumpDir.c_str(), force); + VERB3 log(" RunAnalyzerActions('%s','%s','%s',force=%d)", analyzer.c_str(), package_name, row->db_dump_dir, force); + RunAnalyzerActions(analyzer.c_str(), package_name, row->db_dump_dir, force); free(package_name); - if (DebugDumpToCrashReport(row.m_sDebugDumpDir.c_str(), pCrashData)) + if (DebugDumpToCrashReport(row->db_dump_dir, pCrashData)) { - add_to_crash_data_ext(pCrashData, CD_UUID , CD_SYS, CD_ISNOTEDITABLE, row.m_sUUID.c_str()); + add_to_crash_data_ext(pCrashData, CD_UUID , CD_SYS, CD_ISNOTEDITABLE, row->db_uuid); add_to_crash_data_ext(pCrashData, CD_DUPHASH, CD_TXT, CD_ISNOTEDITABLE, dup_hash.c_str()); } else { + db_row_free(row); error_msg("Error loading crash data"); return MW_ERROR; } + db_row_free(row); } catch (CABRTException& e) { @@ -611,20 +620,23 @@ static bool is_debug_dump_saved(long uid, const char *debug_dump_dir)
CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - vector_database_rows_t rows = database->GetUIDData(uid); + GList *table = database->GetUIDData(uid); database->DisConnect();
- size_t ii; bool found = false; - for (ii = 0; ii < rows.size(); ii++) + struct db_row *row = NULL; + for (GList *li = table; li != NULL; li = g_list_next(li)) { - if (0 == strcmp(rows[ii].m_sDebugDumpDir.c_str(), debug_dump_dir)) + row = (struct db_row*)li->data; + if (0 == strcmp(row->db_dump_dir, debug_dump_dir)) { found = true; break; } }
+ db_list_free(table); + return found; }
@@ -1012,24 +1024,28 @@ static mw_result_t SaveDebugDumpToDatabase(const char *crash_id, database->Connect(); /* note: if [UUID,UID] record exists, pDebugDumpDir is not updated in the record */ database->Insert_or_Update(crash_id, inform_all_users, pDebugDumpDir, pTime); - database_row_t row = database->GetRow(crash_id); + + struct db_row *row = database->GetRow(crash_id); database->DisConnect();
mw_result_t res = FillCrashInfo(crash_id, pCrashData); if (res == MW_OK) { const char *first = get_crash_data_item_content(pCrashData, CD_DUMPDIR).c_str(); - if (row.m_sReported == "1") + if (row && row->db_reported[0] == '1') { log("Crash is in database already (dup of %s) and is reported", first); + db_row_free(row); return MW_REPORTED; } - if (row.m_sCount != "1") + if (row && row->db_count[0] != '1') { + db_row_free(row); log("Crash is in database already (dup of %s)", first); return MW_OCCURRED; } } + db_row_free(row); return res; }
@@ -1122,7 +1138,11 @@ mw_result_t FillCrashInfo(const char *crash_id, { CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - database_row_t row = database->GetRow(crash_id); + + struct db_row *row = database->GetRow(crash_id); + if (!row) + return MW_ERROR; + database->DisConnect();
std::string package; @@ -1131,23 +1151,26 @@ mw_result_t FillCrashInfo(const char *crash_id, std::string analyzer;
dump_dir_t *dd = dd_init(); - if (!dd_opendir(dd, row.m_sDebugDumpDir.c_str())) + if (!dd_opendir(dd, row->db_dump_dir)) { dd_close(dd); + db_row_free(row); return MW_ERROR; }
load_crash_data_from_debug_dump(dd, pCrashData); dd_close(dd);
- add_to_crash_data(pCrashData, CD_UID , row.m_sUID.c_str() ); - add_to_crash_data(pCrashData, CD_UUID , row.m_sUUID.c_str() ); - add_to_crash_data(pCrashData, CD_INFORMALL , row.m_sInformAll.c_str() ); - add_to_crash_data(pCrashData, CD_COUNT , row.m_sCount.c_str() ); - add_to_crash_data(pCrashData, CD_REPORTED , row.m_sReported.c_str() ); - add_to_crash_data(pCrashData, CD_MESSAGE , row.m_sMessage.c_str() ); - add_to_crash_data(pCrashData, CD_DUMPDIR , row.m_sDebugDumpDir.c_str()); - add_to_crash_data(pCrashData, FILENAME_TIME , row.m_sTime.c_str() ); + add_to_crash_data(pCrashData, CD_UID , row->db_uid ); + add_to_crash_data(pCrashData, CD_UUID , row->db_uuid ); + add_to_crash_data(pCrashData, CD_INFORMALL , row->db_inform_all ); + add_to_crash_data(pCrashData, CD_COUNT , row->db_count ); + add_to_crash_data(pCrashData, CD_REPORTED , row->db_reported ); + add_to_crash_data(pCrashData, CD_MESSAGE , row->db_message ); + add_to_crash_data(pCrashData, CD_DUMPDIR , row->db_dump_dir ); + add_to_crash_data(pCrashData, FILENAME_TIME , row->db_time ); + + db_row_free(row);
return MW_OK; } @@ -1155,17 +1178,20 @@ mw_result_t FillCrashInfo(const char *crash_id, void GetUUIDsOfCrash(long caller_uid, vector_string_t &result) { CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); - vector_database_rows_t rows; database->Connect(); - rows = database->GetUIDData(caller_uid); + GList *rows = database->GetUIDData(caller_uid); database->DisConnect();
- unsigned ii; - for (ii = 0; ii < rows.size(); ii++) + struct db_row *row = NULL; + for (GList *li = rows; li != NULL; li = g_list_next(li)) { - string crash_id = ssprintf("%s:%s", rows[ii].m_sUID.c_str(), rows[ii].m_sUUID.c_str()); + row = (struct db_row*)li->data; + string crash_id = ssprintf("%s:%s", row->db_uid, row->db_uuid); result.push_back(crash_id); } + + // TODO: return GList + db_list_free(rows); }
void AddAnalyzerActionOrReporter(const char *pAnalyzer,
On Tue, 2010-09-07 at 18:12 +0200, Nikola Pajkovsky wrote:
+static inline void db_row_free(struct db_row *row) +{
- if (!row)
return;
- free(row->db_uuid);
- free(row->db_uid);
- free(row->db_inform_all);
- free(row->db_dump_dir);
- free(row->db_count);
- free(row->db_reported);
- free(row->db_message);
- free(row->db_time);
- free(row);
+}
+static inline void db_list_free(GList *list) +{
- if (!list)
return;
- struct db_row *row = NULL;
- for (GList *li = list; li != NULL; li = g_list_next(li))
- {
row = (struct db_row*)li->data;
db_row_free(row);
- }
- g_list_free(list);
+}
These two functions should go into a new file, database.c
-static void get_table(vector_database_rows_t& pTable,
sqlite3 *db, const char *fmt, ...)
+static GList *vget_table(sqlite3 *db, const char *fmt, va_list p) {
va_list p;
va_start(p, fmt); char *sql = xvasprintf(fmt, p);
va_end(p);
char **table; int ncol, nrow;
@@ -117,39 +113,54 @@ static void get_table(vector_database_rows_t& pTable, int ret = sqlite3_get_table(db, sql, &table, &nrow, &ncol, &err); if (ret != SQLITE_OK) {
string errstr = ssprintf("Error in SQL:'%s' error: %s", sql, err);
error_msg("Error in SQL:'%s' error: %s", sql, err); free(sql); sqlite3_free(err);
throw CABRTException(EXCEP_PLUGIN, errstr.c_str());
}return (GList*)ERR_PTR;
- VERB2 log("%d rows returned by SQL:%s", nrow, sql);
- VERB2 log("%s: %d rows returned by SQL:%s", __func__, nrow, sql); free(sql);
- pTable.clear();
- GList *rows = NULL; int ii; for (ii = 0; ii < nrow; ii++) { int jj;
database_row_t row;
struct db_row *row = (struct db_row*)xzalloc(sizeof(struct db_row)); for (jj = 0; jj < ncol; jj++) { char *val = table[jj + (ncol*ii) + ncol]; switch (jj) {
case 0: row.m_sUUID = val; break;
case 1: row.m_sUID = val; break;
case 2: row.m_sInformAll = val; break;
case 3: row.m_sDebugDumpDir = val; break;
case 4: row.m_sCount = val; break;
case 5: row.m_sReported = val; break;
case 6: row.m_sTime = val; break;
case 7: row.m_sMessage = val; break;
case 0: row->db_uuid = xstrdup(val); break;
case 1: row->db_uid = xstrdup(val); break;
case 2: row->db_inform_all = xstrdup(val); break;
case 3: row->db_dump_dir = xstrdup(val); break;
case 4: row->db_count = xstrdup(val); break;
case 5: row->db_reported = xstrdup(val); break;
case 6: row->db_time = xstrdup(val); break;
case 7: row->db_message = xstrdup(val); break; } }
In old code, if ncol <= 7, then row.m_sMessage was "". Now it will be NULL. This may need to force you to add checks for NULL in many places.
It's easier to check here, *in one place*, that ncol == 8 (or >= 8?) and therefore all fields are non-NULL:
VERB2 log("%s: %d rows returned by SQL:%s", __func__, nrow, sql); if (ncol < 8) error_msg_and_die("Unexpected number of columns: %d", ncol); ... ... const char *val = table[jj + (ncol*ii) + ncol] ? : ""; switch (jj) ...
This way, you know for sure that every field is non-NULL, and the rest of the code need not be changed to check for NULLs.
- return !table.empty();
- if (!table)
return false;
- db_list_free(table);
- return true;
}
I would write it shorter:
db_list_free(table); return (table != NULL);
but your version is ok too.
@@ -188,27 +188,37 @@ int DeleteDebugDump(const char *crash_id, long caller_uid) { CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect();
database_row_t row = database->GetRow(crash_id);
if (row.m_sUUID == "")
struct db_row *row = database->GetRow(crash_id);
if (!row)
return EIO;
if (!row->db_uuid)
Bug. Should be "if (!row->db_uuid[0])"
{ database->DisConnect();
db_row_free(row); return ENOENT; }
@@ -278,21 +278,27 @@ mw_result_t CreateCrashReport(const char *crash_id, { VERB2 log("CreateCrashReport('%s',%ld,result)", crash_id, caller_uid);
- database_row_t row; CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect();
- row = database->GetRow(crash_id);
- struct db_row *row = database->GetRow(crash_id); database->DisConnect();
- if (row.m_sUUID == "")
- if (row && !row->db_uuid) { error_msg("crash '%s' is not in database", crash_id);
}db_row_free(row); return MW_IN_DB_ERROR;
Same bug.
mw_result_t res = FillCrashInfo(crash_id, pCrashData); if (res == MW_OK) { const char *first = get_crash_data_item_content(pCrashData, CD_DUMPDIR).c_str();
if (row.m_sReported == "1")
if (row && row->db_reported[0] == '1')
This is not the same logic, but in this case it is ok (because db_reported is bool).
{ log("Crash is in database already (dup of %s) and is reported", first);
db_row_free(row); return MW_REPORTED; }
if (row.m_sCount != "1")
if (row && row->db_count[0] != '1')
Bug. db_count of "11" will be misinterpreted as "1" here.
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com --- configure.ac | 2 +- inc/abrtlib.h | 3 + inc/database.h | 51 +++++++++++------- lib/plugins/Makefile.am | 4 +- lib/plugins/SQLite3.cpp | 127 +++++++++++++++++++++++++------------------ lib/plugins/SQLite3.h | 5 +- lib/utils/Makefile.am | 3 +- src/daemon/CrashWatcher.cpp | 24 ++++++--- src/daemon/MiddleWare.cpp | 94 ++++++++++++++++++++------------ 9 files changed, 195 insertions(+), 118 deletions(-)
diff --git a/configure.ac b/configure.ac index cd21bab..3736399 100644 --- a/configure.ac +++ b/configure.ac @@ -16,7 +16,7 @@ AC_SYS_LARGEFILE CXXFLAGS="$CXXFLAGS -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ -D_FILE_OFFSET_BITS=64 -fno-strict-aliasing " CFLAGS="$CFLAGS -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ - -D_FILE_OFFSET_BITS=64 -fno-strict-aliasing " + -D_FILE_OFFSET_BITS=64 -fno-strict-aliasing -std=gnu99 -Wall"
AC_CHECK_PROGS(YACC, 'bison -y', [no]) [if test "$YACC" = "no"] diff --git a/inc/abrtlib.h b/inc/abrtlib.h index 2fc5101..a1ad9c3 100644 --- a/inc/abrtlib.h +++ b/inc/abrtlib.h @@ -64,6 +64,9 @@ int vdprintf(int d, const char *format, va_list ap); #undef NORETURN #define NORETURN __attribute__ ((noreturn))
+#undef ERR_PTR +#define ERR_PTR ((void*)(uintptr_t)1) + #undef ARRAY_SIZE #define ARRAY_SIZE(x) ((unsigned)(sizeof(x) / sizeof((x)[0])))
diff --git a/inc/database.h b/inc/database.h index df109bd..5d5bcf0 100644 --- a/inc/database.h +++ b/inc/database.h @@ -21,9 +21,7 @@ #ifndef DATABASE_H_ #define DATABASE_H_
-#include <string> -#include <vector> -#include "plugin.h" +#include <glib.h>
/** * Table @@ -33,25 +31,39 @@ * primary key (UUID, UID) */
+#ifdef __cplusplus +extern "C" { +#endif + /** * A struct contains one database row. */ -typedef struct database_row_t +struct db_row { - std::string m_sUUID; /**< A local UUID.*/ - std::string m_sUID; /**< An UID of an user.*/ - std::string m_sInformAll; - std::string m_sDebugDumpDir; /**< A debugdump directory of a crash.*/ - std::string m_sCount; /**< Crash rate.*/ - std::string m_sReported; /**< Is a row reported?*/ - std::string m_sMessage; /**< if a row is reported, then there can be store message abotu that*/ - std::string m_sTime; /**< Time of last occurred crash with same local UUID*/ -} database_row_t; + char *db_uuid; /**< A local UUID.*/ + char *db_uid; /**< An UID of an user.*/ + char *db_inform_all; + char *db_dump_dir; /**< A debugdump directory of a crash.*/ + char *db_count; /**< Crash rate.*/ + char *db_reported; /**< Is a row reported?*/ + char *db_message; /**< if a row is reported, then there can be store message abotu that*/ + char *db_time; /**< Time of last occurred crash with same local UUID*/ +};
-/** - * A vector contains one or more database rows. - */ -typedef std::vector<database_row_t> vector_database_rows_t; +void db_row_free(struct db_row *row); + +void db_list_free(GList *list); + +#ifdef __cplusplus +} +#endif + +#ifdef __cplusplus + +#include <string> +#include <vector> + +#include "plugin.h"
/** * An abstract class. The class defines a database plugin interface. @@ -104,7 +116,7 @@ class CDatabase : public CPlugin * @param pUID An UID of an user. * @return A vector of matched rows. */ - virtual vector_database_rows_t GetUIDData(long caller_uid) = 0; + virtual GList *GetUIDData(long caller_uid) = 0; /** * A method, which returns one row accordind to UUID of a crash and * UID of an user. If there are no row, empty row is returned. @@ -112,7 +124,8 @@ class CDatabase : public CPlugin * @param pUID An UID of an user. * @return A matched row. */ - virtual database_row_t GetRow(const char *crash_id) = 0; + virtual struct db_row *GetRow(const char *crash_id) = 0; }; +#endif
#endif diff --git a/lib/plugins/Makefile.am b/lib/plugins/Makefile.am index a896e14..ca874e1 100644 --- a/lib/plugins/Makefile.am +++ b/lib/plugins/Makefile.am @@ -98,8 +98,8 @@ libMailx_la_CPPFLAGS = -I$(INC_PATH) -I$(UTILS_PATH) -DPLUGINS_LIB_DIR="$(PLUGI # SQLite3 libSQLite3_la_SOURCES = SQLite3.cpp SQLite3.h libSQLite3_la_LDFLAGS = -avoid-version -libSQLite3_la_LIBADD = $(SQLITE3_LIBS) -libSQLite3_la_CPPFLAGS = -I$(INC_PATH) -I$(UTILS_PATH) $(SQLITE3_CFLAGS) -DLOCALSTATEDIR='"$(localstatedir)"' +libSQLite3_la_LIBADD = $(SQLITE3_LIBS) $(GLIB_LIBS) +libSQLite3_la_CPPFLAGS = -I$(INC_PATH) -I$(UTILS_PATH) $(SQLITE3_CFLAGS) -DLOCALSTATEDIR='"$(localstatedir)"' $(GLIB_CFLAGS)
# Logger libLogger_la_SOURCES = Logger.cpp Logger.h diff --git a/lib/plugins/SQLite3.cpp b/lib/plugins/SQLite3.cpp index 322f15a..040314f 100644 --- a/lib/plugins/SQLite3.cpp +++ b/lib/plugins/SQLite3.cpp @@ -42,6 +42,8 @@ using namespace std;
#define COL_REPORTER "Reporter"
+#define NUM_COL 8 + /* Is this string safe wrt SQL injection? * PHP's mysql_real_escape_string() treats , ', ", \x00, \n, \r, and \x1a as special. * We are a bit more paranoid and disallow any control chars. @@ -103,13 +105,9 @@ static string sql_escape(const char *str) /* Note: * expects "SELECT * FROM ...", not "SELECT <only some fields> FROM ..." */ -static void get_table(vector_database_rows_t& pTable, - sqlite3 *db, const char *fmt, ...) +static GList *vget_table(sqlite3 *db, const char *fmt, va_list p) { - va_list p; - va_start(p, fmt); char *sql = xvasprintf(fmt, p); - va_end(p);
char **table; int ncol, nrow; @@ -117,39 +115,57 @@ static void get_table(vector_database_rows_t& pTable, int ret = sqlite3_get_table(db, sql, &table, &nrow, &ncol, &err); if (ret != SQLITE_OK) { - string errstr = ssprintf("Error in SQL:'%s' error: %s", sql, err); + error_msg("Error in SQL:'%s' error: %s", sql, err); free(sql); sqlite3_free(err); - throw CABRTException(EXCEP_PLUGIN, errstr.c_str()); + return (GList*)ERR_PTR; } - VERB2 log("%d rows returned by SQL:%s", nrow, sql); + VERB2 log("%s: %d rows returned by SQL:%s", __func__, nrow, sql); free(sql);
- pTable.clear(); + if (ncol < NUM_COL) + error_msg_and_die("Unexpected number of columns: %d", ncol); + + GList *rows = NULL; int ii; for (ii = 0; ii < nrow; ii++) { int jj; - database_row_t row; + struct db_row *row = (struct db_row*)xzalloc(sizeof(struct db_row)); for (jj = 0; jj < ncol; jj++) { char *val = table[jj + (ncol*ii) + ncol]; switch (jj) { - case 0: row.m_sUUID = val; break; - case 1: row.m_sUID = val; break; - case 2: row.m_sInformAll = val; break; - case 3: row.m_sDebugDumpDir = val; break; - case 4: row.m_sCount = val; break; - case 5: row.m_sReported = val; break; - case 6: row.m_sTime = val; break; - case 7: row.m_sMessage = val; break; + case 0: row->db_uuid = xstrdup(val); break; + case 1: row->db_uid = xstrdup(val); break; + case 2: row->db_inform_all = xstrdup(val); break; + case 3: row->db_dump_dir = xstrdup(val); break; + case 4: row->db_count = xstrdup(val); break; + case 5: row->db_reported = xstrdup(val); break; + case 6: row->db_time = xstrdup(val); break; + case 7: row->db_message = xstrdup(val); break; } } - pTable.push_back(row); + rows = g_list_append(rows, row);
} sqlite3_free_table(table); + + return rows; +} + +static GList *get_table_or_die(sqlite3 *db, const char *fmt, ...) +{ + va_list p; + va_start(p, fmt); + GList *table = vget_table(db, fmt, p); + va_end(p); + + if (table == (GList*)ERR_PTR) + xfunc_die(); + + return table; }
static int execute_sql(sqlite3 *db, const char *fmt, ...) @@ -177,13 +193,17 @@ static int execute_sql(sqlite3 *db, const char *fmt, ...)
static bool exists_uuid_uid(sqlite3 *db, const char *UUID, const char *UID) { - vector_database_rows_t table; - get_table(table, db, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", - UUID, UID + GList *table = get_table_or_die(db, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", + UUID, UID ); - return !table.empty(); + + if (!table) + return false; + + db_list_free(table); + + return true; }
static void update_from_old_ver(sqlite3 *db, int old_version) @@ -496,27 +516,26 @@ void CSQLite3::DeleteRows_by_dir(const char *dump_dir) }
/* Get UID:UUID pair(s) to delete */ - vector_database_rows_t table; - get_table(table, m_pDB, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_DEBUG_DUMP_PATH"='%s';", - dump_dir + GList *table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_DEBUG_DUMP_PATH"='%s';", + dump_dir ); - if (table.empty()) + + if (!table) { return; }
+ struct db_row *row = NULL; /* Delete from both tables */ - vector_database_rows_t::iterator it = table.begin(); - while (it != table.end()) + for (GList *li = table; li != NULL; li = g_list_next(li)) { + row = (struct db_row*)li->data; execute_sql(m_pDB, "DELETE FROM "ABRT_REPRESULT_TABLE " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", - it->m_sUUID.c_str(), it->m_sUID.c_str() + row->db_uuid, row->db_uid ); - it++; } execute_sql(m_pDB, "DELETE FROM "ABRT_TABLE @@ -601,32 +620,31 @@ void CSQLite3::SetReportedPerReporter(const char *crash_id, } }
-vector_database_rows_t CSQLite3::GetUIDData(long caller_uid) +GList *CSQLite3::GetUIDData(long caller_uid) { - vector_database_rows_t table; + GList *table = NULL;
if (caller_uid == 0) { - get_table(table, m_pDB, "SELECT * FROM "ABRT_TABLE";"); + table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE";"); } else { - get_table(table, m_pDB, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_UID"='%ld' OR "COL_INFORMALL"=1;", - caller_uid + table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_UID"='%ld' OR "COL_INFORMALL"=1;", + caller_uid ); } return table; }
-database_row_t CSQLite3::GetRow(const char *crash_id) +struct db_row *CSQLite3::GetRow(const char *crash_id) { const char *UUID = strchr(crash_id, ':'); if (!UUID || !is_string_safe(crash_id) ) { - return database_row_t(); + return NULL; }
/* Split crash_id into UID:UUID */ @@ -636,18 +654,23 @@ database_row_t CSQLite3::GetRow(const char *crash_id) strncpy(UID, crash_id, uid_len); UID[uid_len] = '\0';
- vector_database_rows_t table; - get_table(table, m_pDB, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", - UUID, UID + GList *table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", + UUID, UID );
- if (table.size() == 0) + if (!table) { - return database_row_t(); + return NULL; } - return table[0]; + + GList *first = g_list_first(table); + struct db_row *row = (struct db_row*)xzalloc(sizeof(struct db_row)); + memcpy(row, first->data, sizeof(struct db_row)); + + db_list_free(table); + + return row; }
void CSQLite3::SetSettings(const map_plugin_settings_t& pSettings) diff --git a/lib/plugins/SQLite3.h b/lib/plugins/SQLite3.h index d2af864..3dfd0a9 100644 --- a/lib/plugins/SQLite3.h +++ b/lib/plugins/SQLite3.h @@ -47,10 +47,11 @@ class CSQLite3 : public CDatabase virtual void SetReportedPerReporter(const char *crash_id, const char *reporter, const char *pMessage); - virtual vector_database_rows_t GetUIDData(long caller_uid); - virtual database_row_t GetRow(const char *crash_id); + virtual GList *GetUIDData(long caller_uid); + virtual struct db_row *GetRow(const char *crash_id);
virtual void SetSettings(const map_plugin_settings_t& pSettings); };
+ #endif diff --git a/lib/utils/Makefile.am b/lib/utils/Makefile.am index cc77c82..15fd03e 100644 --- a/lib/utils/Makefile.am +++ b/lib/utils/Makefile.am @@ -31,7 +31,8 @@ libABRTUtils_la_SOURCES = \ backtrace_parser.y \ strbuf.c strbuf.h \ abrt_packages.c abrt_packages.h \ - hooklib.c hooklib.h + hooklib.c hooklib.h \ + database.c libABRTUtils_la_CPPFLAGS = \ -Wall -Werror \ -I$(srcdir)/../../inc \ diff --git a/src/daemon/CrashWatcher.cpp b/src/daemon/CrashWatcher.cpp index 6310118..fbb9de3 100644 --- a/src/daemon/CrashWatcher.cpp +++ b/src/daemon/CrashWatcher.cpp @@ -188,27 +188,37 @@ int DeleteDebugDump(const char *crash_id, long caller_uid) { CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - database_row_t row = database->GetRow(crash_id); - if (row.m_sUUID == "") + struct db_row *row = database->GetRow(crash_id); + if (!row) + return EIO; + + if (!row->db_uuid[0]) { database->DisConnect(); + db_row_free(row); return ENOENT; } + + char caller_uid_str[sizeof(long) * 3 + 2]; + sprintf(caller_uid_str, "%li", caller_uid); + if (caller_uid != 0 /* not called by root */ - && row.m_sInformAll != "1" - && to_string(caller_uid) != row.m_sUID + && row->db_inform_all[0] != '1' + && strcmp(caller_uid_str, row->db_uid) != 0 ) { database->DisConnect(); + db_row_free(row); return EPERM; } database->DeleteRow(crash_id); database->DisConnect(); - const char *dump_dir = row.m_sDebugDumpDir.c_str(); - if (dump_dir[0] != '\0') + if (row->db_dump_dir[0] != '\0') { - delete_debug_dump_dir(dump_dir); + delete_debug_dump_dir(row->db_dump_dir); + db_row_free(row); return 0; /* success */ } + db_row_free(row); } catch (CABRTException& e) { diff --git a/src/daemon/MiddleWare.cpp b/src/daemon/MiddleWare.cpp index 7752dc3..7de0049 100644 --- a/src/daemon/MiddleWare.cpp +++ b/src/daemon/MiddleWare.cpp @@ -278,21 +278,27 @@ mw_result_t CreateCrashReport(const char *crash_id, { VERB2 log("CreateCrashReport('%s',%ld,result)", crash_id, caller_uid);
- database_row_t row; CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - row = database->GetRow(crash_id); + + struct db_row *row = database->GetRow(crash_id); database->DisConnect(); - if (row.m_sUUID == "") + if (!row->db_uuid[0]) { error_msg("crash '%s' is not in database", crash_id); + db_row_free(row); return MW_IN_DB_ERROR; } + + char caller_uid_str[sizeof(long) * 3 + 2]; + sprintf(caller_uid_str, "%li", caller_uid); + if (caller_uid != 0 /* not called by root */ - && row.m_sInformAll != "1" - && to_string(caller_uid) != row.m_sUID + && row->db_inform_all[0] != '1' + && strcmp(caller_uid_str, row->db_uid) != 0 ) { error_msg("crash '%s' can't be accessed by user with uid %ld", crash_id, caller_uid); + db_row_free(row); return MW_IN_DB_ERROR; }
@@ -300,9 +306,10 @@ mw_result_t CreateCrashReport(const char *crash_id, try { dump_dir_t *dd = dd_init(); - if (!dd_opendir(dd, row.m_sDebugDumpDir.c_str())) + if (!dd_opendir(dd, row->db_dump_dir)) { dd_close(dd); + db_row_free(row); return MW_ERROR; }
@@ -319,24 +326,26 @@ mw_result_t CreateCrashReport(const char *crash_id, // Why do we reload dump dir's data via DebugDumpToCrashReport?
VERB3 log(" run_analyser_CreateReport('%s')", analyzer.c_str()); - run_analyser_CreateReport(analyzer.c_str(), row.m_sDebugDumpDir.c_str(), force); + run_analyser_CreateReport(analyzer.c_str(), row->db_dump_dir, force);
- std::string dup_hash = GetGlobalUUID(analyzer.c_str(), row.m_sDebugDumpDir.c_str()); + std::string dup_hash = GetGlobalUUID(analyzer.c_str(), row->db_dump_dir); VERB3 log(" DUPHASH:'%s'", dup_hash.c_str());
- VERB3 log(" RunAnalyzerActions('%s','%s','%s',force=%d)", analyzer.c_str(), package_name, row.m_sDebugDumpDir.c_str(), force); - RunAnalyzerActions(analyzer.c_str(), package_name, row.m_sDebugDumpDir.c_str(), force); + VERB3 log(" RunAnalyzerActions('%s','%s','%s',force=%d)", analyzer.c_str(), package_name, row->db_dump_dir, force); + RunAnalyzerActions(analyzer.c_str(), package_name, row->db_dump_dir, force); free(package_name); - if (DebugDumpToCrashReport(row.m_sDebugDumpDir.c_str(), pCrashData)) + if (DebugDumpToCrashReport(row->db_dump_dir, pCrashData)) { - add_to_crash_data_ext(pCrashData, CD_UUID , CD_SYS, CD_ISNOTEDITABLE, row.m_sUUID.c_str()); + add_to_crash_data_ext(pCrashData, CD_UUID , CD_SYS, CD_ISNOTEDITABLE, row->db_uuid); add_to_crash_data_ext(pCrashData, CD_DUPHASH, CD_TXT, CD_ISNOTEDITABLE, dup_hash.c_str()); } else { + db_row_free(row); error_msg("Error loading crash data"); return MW_ERROR; } + db_row_free(row); } catch (CABRTException& e) { @@ -611,20 +620,23 @@ static bool is_debug_dump_saved(long uid, const char *debug_dump_dir)
CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - vector_database_rows_t rows = database->GetUIDData(uid); + GList *table = database->GetUIDData(uid); database->DisConnect();
- size_t ii; bool found = false; - for (ii = 0; ii < rows.size(); ii++) + struct db_row *row = NULL; + for (GList *li = table; li != NULL; li = g_list_next(li)) { - if (0 == strcmp(rows[ii].m_sDebugDumpDir.c_str(), debug_dump_dir)) + row = (struct db_row*)li->data; + if (0 == strcmp(row->db_dump_dir, debug_dump_dir)) { found = true; break; } }
+ db_list_free(table); + return found; }
@@ -1012,24 +1024,28 @@ static mw_result_t SaveDebugDumpToDatabase(const char *crash_id, database->Connect(); /* note: if [UUID,UID] record exists, pDebugDumpDir is not updated in the record */ database->Insert_or_Update(crash_id, inform_all_users, pDebugDumpDir, pTime); - database_row_t row = database->GetRow(crash_id); + + struct db_row *row = database->GetRow(crash_id); database->DisConnect();
mw_result_t res = FillCrashInfo(crash_id, pCrashData); if (res == MW_OK) { const char *first = get_crash_data_item_content(pCrashData, CD_DUMPDIR).c_str(); - if (row.m_sReported == "1") + if (row && row->db_reported[0] == '1') { log("Crash is in database already (dup of %s) and is reported", first); + db_row_free(row); return MW_REPORTED; } - if (row.m_sCount != "1") + if (row && xatou(row->db_count) > 1) { + db_row_free(row); log("Crash is in database already (dup of %s)", first); return MW_OCCURRED; } } + db_row_free(row); return res; }
@@ -1122,7 +1138,11 @@ mw_result_t FillCrashInfo(const char *crash_id, { CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - database_row_t row = database->GetRow(crash_id); + + struct db_row *row = database->GetRow(crash_id); + if (!row) + return MW_ERROR; + database->DisConnect();
std::string package; @@ -1131,23 +1151,26 @@ mw_result_t FillCrashInfo(const char *crash_id, std::string analyzer;
dump_dir_t *dd = dd_init(); - if (!dd_opendir(dd, row.m_sDebugDumpDir.c_str())) + if (!dd_opendir(dd, row->db_dump_dir)) { dd_close(dd); + db_row_free(row); return MW_ERROR; }
load_crash_data_from_debug_dump(dd, pCrashData); dd_close(dd);
- add_to_crash_data(pCrashData, CD_UID , row.m_sUID.c_str() ); - add_to_crash_data(pCrashData, CD_UUID , row.m_sUUID.c_str() ); - add_to_crash_data(pCrashData, CD_INFORMALL , row.m_sInformAll.c_str() ); - add_to_crash_data(pCrashData, CD_COUNT , row.m_sCount.c_str() ); - add_to_crash_data(pCrashData, CD_REPORTED , row.m_sReported.c_str() ); - add_to_crash_data(pCrashData, CD_MESSAGE , row.m_sMessage.c_str() ); - add_to_crash_data(pCrashData, CD_DUMPDIR , row.m_sDebugDumpDir.c_str()); - add_to_crash_data(pCrashData, FILENAME_TIME , row.m_sTime.c_str() ); + add_to_crash_data(pCrashData, CD_UID , row->db_uid ); + add_to_crash_data(pCrashData, CD_UUID , row->db_uuid ); + add_to_crash_data(pCrashData, CD_INFORMALL , row->db_inform_all ); + add_to_crash_data(pCrashData, CD_COUNT , row->db_count ); + add_to_crash_data(pCrashData, CD_REPORTED , row->db_reported ); + add_to_crash_data(pCrashData, CD_MESSAGE , row->db_message ); + add_to_crash_data(pCrashData, CD_DUMPDIR , row->db_dump_dir ); + add_to_crash_data(pCrashData, FILENAME_TIME , row->db_time ); + + db_row_free(row);
return MW_OK; } @@ -1155,17 +1178,20 @@ mw_result_t FillCrashInfo(const char *crash_id, void GetUUIDsOfCrash(long caller_uid, vector_string_t &result) { CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); - vector_database_rows_t rows; database->Connect(); - rows = database->GetUIDData(caller_uid); + GList *rows = database->GetUIDData(caller_uid); database->DisConnect();
- unsigned ii; - for (ii = 0; ii < rows.size(); ii++) + struct db_row *row = NULL; + for (GList *li = rows; li != NULL; li = g_list_next(li)) { - string crash_id = ssprintf("%s:%s", rows[ii].m_sUID.c_str(), rows[ii].m_sUUID.c_str()); + row = (struct db_row*)li->data; + string crash_id = ssprintf("%s:%s", row->db_uid, row->db_uuid); result.push_back(crash_id); } + + // TODO: return GList + db_list_free(rows); }
void AddAnalyzerActionOrReporter(const char *pAnalyzer,
On Wed, 2010-09-08 at 14:39 +0200, Nikola Pajkovsky wrote:
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com
configure.ac | 2 +- inc/abrtlib.h | 3 + inc/database.h | 51 +++++++++++------- lib/plugins/Makefile.am | 4 +- lib/plugins/SQLite3.cpp | 127 +++++++++++++++++++++++++------------------ lib/plugins/SQLite3.h | 5 +- lib/utils/Makefile.am | 3 +- src/daemon/CrashWatcher.cpp | 24 ++++++--- src/daemon/MiddleWare.cpp | 94 ++++++++++++++++++++------------ 9 files changed, 195 insertions(+), 118 deletions(-)
--- a/src/daemon/CrashWatcher.cpp +++ b/src/daemon/CrashWatcher.cpp @@ -188,27 +188,37 @@ int DeleteDebugDump(const char *crash_id, long caller_uid) { CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect();
database_row_t row = database->GetRow(crash_id);
if (row.m_sUUID == "")
struct db_row *row = database->GetRow(crash_id);
if (!row)
return EIO;
if (!row->db_uuid[0]) { database->DisConnect();
db_row_free(row); return ENOENT; }
Looking at it more, I think old code was checking that there is actually no such row. Old code couldn't return NULL, so it was returning an empty record. New code returns NULL, therefore new code should do "if (!row)" check here (and drop db_row_free(row) in the if body).
- struct db_row *row = database->GetRow(crash_id); database->DisConnect();
- if (row.m_sUUID == "")
- if (!row->db_uuid[0]) { error_msg("crash '%s' is not in database", crash_id);
}db_row_free(row); return MW_IN_DB_ERROR;
Same here.
- size_t ii; bool found = false;
- for (ii = 0; ii < rows.size(); ii++)
- struct db_row *row = NULL;
This line can be moved inside the loop:
- for (GList *li = table; li != NULL; li = g_list_next(li)) {
if (0 == strcmp(rows[ii].m_sDebugDumpDir.c_str(), debug_dump_dir))
row = (struct db_row*)li->data;
to here ^^^^^^^^^^^^^:
}if (0 == strcmp(row->db_dump_dir, debug_dump_dir)) { found = true; break; }
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com --- configure.ac | 2 +- inc/abrtlib.h | 3 + inc/database.h | 51 +++++++++++------- lib/plugins/Makefile.am | 4 +- lib/plugins/SQLite3.cpp | 127 +++++++++++++++++++++++++------------------ lib/plugins/SQLite3.h | 5 +- lib/utils/Makefile.am | 3 +- lib/utils/database.c | 52 ++++++++++++++++++ src/daemon/CrashWatcher.cpp | 20 +++++--- src/daemon/MiddleWare.cpp | 93 ++++++++++++++++++++------------ 10 files changed, 242 insertions(+), 118 deletions(-) create mode 100644 lib/utils/database.c
diff --git a/configure.ac b/configure.ac index cd21bab..3736399 100644 --- a/configure.ac +++ b/configure.ac @@ -16,7 +16,7 @@ AC_SYS_LARGEFILE CXXFLAGS="$CXXFLAGS -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ -D_FILE_OFFSET_BITS=64 -fno-strict-aliasing " CFLAGS="$CFLAGS -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE \ - -D_FILE_OFFSET_BITS=64 -fno-strict-aliasing " + -D_FILE_OFFSET_BITS=64 -fno-strict-aliasing -std=gnu99 -Wall"
AC_CHECK_PROGS(YACC, 'bison -y', [no]) [if test "$YACC" = "no"] diff --git a/inc/abrtlib.h b/inc/abrtlib.h index 2fc5101..a1ad9c3 100644 --- a/inc/abrtlib.h +++ b/inc/abrtlib.h @@ -64,6 +64,9 @@ int vdprintf(int d, const char *format, va_list ap); #undef NORETURN #define NORETURN __attribute__ ((noreturn))
+#undef ERR_PTR +#define ERR_PTR ((void*)(uintptr_t)1) + #undef ARRAY_SIZE #define ARRAY_SIZE(x) ((unsigned)(sizeof(x) / sizeof((x)[0])))
diff --git a/inc/database.h b/inc/database.h index df109bd..5d5bcf0 100644 --- a/inc/database.h +++ b/inc/database.h @@ -21,9 +21,7 @@ #ifndef DATABASE_H_ #define DATABASE_H_
-#include <string> -#include <vector> -#include "plugin.h" +#include <glib.h>
/** * Table @@ -33,25 +31,39 @@ * primary key (UUID, UID) */
+#ifdef __cplusplus +extern "C" { +#endif + /** * A struct contains one database row. */ -typedef struct database_row_t +struct db_row { - std::string m_sUUID; /**< A local UUID.*/ - std::string m_sUID; /**< An UID of an user.*/ - std::string m_sInformAll; - std::string m_sDebugDumpDir; /**< A debugdump directory of a crash.*/ - std::string m_sCount; /**< Crash rate.*/ - std::string m_sReported; /**< Is a row reported?*/ - std::string m_sMessage; /**< if a row is reported, then there can be store message abotu that*/ - std::string m_sTime; /**< Time of last occurred crash with same local UUID*/ -} database_row_t; + char *db_uuid; /**< A local UUID.*/ + char *db_uid; /**< An UID of an user.*/ + char *db_inform_all; + char *db_dump_dir; /**< A debugdump directory of a crash.*/ + char *db_count; /**< Crash rate.*/ + char *db_reported; /**< Is a row reported?*/ + char *db_message; /**< if a row is reported, then there can be store message abotu that*/ + char *db_time; /**< Time of last occurred crash with same local UUID*/ +};
-/** - * A vector contains one or more database rows. - */ -typedef std::vector<database_row_t> vector_database_rows_t; +void db_row_free(struct db_row *row); + +void db_list_free(GList *list); + +#ifdef __cplusplus +} +#endif + +#ifdef __cplusplus + +#include <string> +#include <vector> + +#include "plugin.h"
/** * An abstract class. The class defines a database plugin interface. @@ -104,7 +116,7 @@ class CDatabase : public CPlugin * @param pUID An UID of an user. * @return A vector of matched rows. */ - virtual vector_database_rows_t GetUIDData(long caller_uid) = 0; + virtual GList *GetUIDData(long caller_uid) = 0; /** * A method, which returns one row accordind to UUID of a crash and * UID of an user. If there are no row, empty row is returned. @@ -112,7 +124,8 @@ class CDatabase : public CPlugin * @param pUID An UID of an user. * @return A matched row. */ - virtual database_row_t GetRow(const char *crash_id) = 0; + virtual struct db_row *GetRow(const char *crash_id) = 0; }; +#endif
#endif diff --git a/lib/plugins/Makefile.am b/lib/plugins/Makefile.am index a896e14..ca874e1 100644 --- a/lib/plugins/Makefile.am +++ b/lib/plugins/Makefile.am @@ -98,8 +98,8 @@ libMailx_la_CPPFLAGS = -I$(INC_PATH) -I$(UTILS_PATH) -DPLUGINS_LIB_DIR="$(PLUGI # SQLite3 libSQLite3_la_SOURCES = SQLite3.cpp SQLite3.h libSQLite3_la_LDFLAGS = -avoid-version -libSQLite3_la_LIBADD = $(SQLITE3_LIBS) -libSQLite3_la_CPPFLAGS = -I$(INC_PATH) -I$(UTILS_PATH) $(SQLITE3_CFLAGS) -DLOCALSTATEDIR='"$(localstatedir)"' +libSQLite3_la_LIBADD = $(SQLITE3_LIBS) $(GLIB_LIBS) +libSQLite3_la_CPPFLAGS = -I$(INC_PATH) -I$(UTILS_PATH) $(SQLITE3_CFLAGS) -DLOCALSTATEDIR='"$(localstatedir)"' $(GLIB_CFLAGS)
# Logger libLogger_la_SOURCES = Logger.cpp Logger.h diff --git a/lib/plugins/SQLite3.cpp b/lib/plugins/SQLite3.cpp index 322f15a..040314f 100644 --- a/lib/plugins/SQLite3.cpp +++ b/lib/plugins/SQLite3.cpp @@ -42,6 +42,8 @@ using namespace std;
#define COL_REPORTER "Reporter"
+#define NUM_COL 8 + /* Is this string safe wrt SQL injection? * PHP's mysql_real_escape_string() treats , ', ", \x00, \n, \r, and \x1a as special. * We are a bit more paranoid and disallow any control chars. @@ -103,13 +105,9 @@ static string sql_escape(const char *str) /* Note: * expects "SELECT * FROM ...", not "SELECT <only some fields> FROM ..." */ -static void get_table(vector_database_rows_t& pTable, - sqlite3 *db, const char *fmt, ...) +static GList *vget_table(sqlite3 *db, const char *fmt, va_list p) { - va_list p; - va_start(p, fmt); char *sql = xvasprintf(fmt, p); - va_end(p);
char **table; int ncol, nrow; @@ -117,39 +115,57 @@ static void get_table(vector_database_rows_t& pTable, int ret = sqlite3_get_table(db, sql, &table, &nrow, &ncol, &err); if (ret != SQLITE_OK) { - string errstr = ssprintf("Error in SQL:'%s' error: %s", sql, err); + error_msg("Error in SQL:'%s' error: %s", sql, err); free(sql); sqlite3_free(err); - throw CABRTException(EXCEP_PLUGIN, errstr.c_str()); + return (GList*)ERR_PTR; } - VERB2 log("%d rows returned by SQL:%s", nrow, sql); + VERB2 log("%s: %d rows returned by SQL:%s", __func__, nrow, sql); free(sql);
- pTable.clear(); + if (ncol < NUM_COL) + error_msg_and_die("Unexpected number of columns: %d", ncol); + + GList *rows = NULL; int ii; for (ii = 0; ii < nrow; ii++) { int jj; - database_row_t row; + struct db_row *row = (struct db_row*)xzalloc(sizeof(struct db_row)); for (jj = 0; jj < ncol; jj++) { char *val = table[jj + (ncol*ii) + ncol]; switch (jj) { - case 0: row.m_sUUID = val; break; - case 1: row.m_sUID = val; break; - case 2: row.m_sInformAll = val; break; - case 3: row.m_sDebugDumpDir = val; break; - case 4: row.m_sCount = val; break; - case 5: row.m_sReported = val; break; - case 6: row.m_sTime = val; break; - case 7: row.m_sMessage = val; break; + case 0: row->db_uuid = xstrdup(val); break; + case 1: row->db_uid = xstrdup(val); break; + case 2: row->db_inform_all = xstrdup(val); break; + case 3: row->db_dump_dir = xstrdup(val); break; + case 4: row->db_count = xstrdup(val); break; + case 5: row->db_reported = xstrdup(val); break; + case 6: row->db_time = xstrdup(val); break; + case 7: row->db_message = xstrdup(val); break; } } - pTable.push_back(row); + rows = g_list_append(rows, row);
} sqlite3_free_table(table); + + return rows; +} + +static GList *get_table_or_die(sqlite3 *db, const char *fmt, ...) +{ + va_list p; + va_start(p, fmt); + GList *table = vget_table(db, fmt, p); + va_end(p); + + if (table == (GList*)ERR_PTR) + xfunc_die(); + + return table; }
static int execute_sql(sqlite3 *db, const char *fmt, ...) @@ -177,13 +193,17 @@ static int execute_sql(sqlite3 *db, const char *fmt, ...)
static bool exists_uuid_uid(sqlite3 *db, const char *UUID, const char *UID) { - vector_database_rows_t table; - get_table(table, db, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", - UUID, UID + GList *table = get_table_or_die(db, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", + UUID, UID ); - return !table.empty(); + + if (!table) + return false; + + db_list_free(table); + + return true; }
static void update_from_old_ver(sqlite3 *db, int old_version) @@ -496,27 +516,26 @@ void CSQLite3::DeleteRows_by_dir(const char *dump_dir) }
/* Get UID:UUID pair(s) to delete */ - vector_database_rows_t table; - get_table(table, m_pDB, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_DEBUG_DUMP_PATH"='%s';", - dump_dir + GList *table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_DEBUG_DUMP_PATH"='%s';", + dump_dir ); - if (table.empty()) + + if (!table) { return; }
+ struct db_row *row = NULL; /* Delete from both tables */ - vector_database_rows_t::iterator it = table.begin(); - while (it != table.end()) + for (GList *li = table; li != NULL; li = g_list_next(li)) { + row = (struct db_row*)li->data; execute_sql(m_pDB, "DELETE FROM "ABRT_REPRESULT_TABLE " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", - it->m_sUUID.c_str(), it->m_sUID.c_str() + row->db_uuid, row->db_uid ); - it++; } execute_sql(m_pDB, "DELETE FROM "ABRT_TABLE @@ -601,32 +620,31 @@ void CSQLite3::SetReportedPerReporter(const char *crash_id, } }
-vector_database_rows_t CSQLite3::GetUIDData(long caller_uid) +GList *CSQLite3::GetUIDData(long caller_uid) { - vector_database_rows_t table; + GList *table = NULL;
if (caller_uid == 0) { - get_table(table, m_pDB, "SELECT * FROM "ABRT_TABLE";"); + table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE";"); } else { - get_table(table, m_pDB, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_UID"='%ld' OR "COL_INFORMALL"=1;", - caller_uid + table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_UID"='%ld' OR "COL_INFORMALL"=1;", + caller_uid ); } return table; }
-database_row_t CSQLite3::GetRow(const char *crash_id) +struct db_row *CSQLite3::GetRow(const char *crash_id) { const char *UUID = strchr(crash_id, ':'); if (!UUID || !is_string_safe(crash_id) ) { - return database_row_t(); + return NULL; }
/* Split crash_id into UID:UUID */ @@ -636,18 +654,23 @@ database_row_t CSQLite3::GetRow(const char *crash_id) strncpy(UID, crash_id, uid_len); UID[uid_len] = '\0';
- vector_database_rows_t table; - get_table(table, m_pDB, - "SELECT * FROM "ABRT_TABLE - " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", - UUID, UID + GList *table = get_table_or_die(m_pDB, "SELECT * FROM "ABRT_TABLE + " WHERE "COL_UUID"='%s' AND "COL_UID"='%s';", + UUID, UID );
- if (table.size() == 0) + if (!table) { - return database_row_t(); + return NULL; } - return table[0]; + + GList *first = g_list_first(table); + struct db_row *row = (struct db_row*)xzalloc(sizeof(struct db_row)); + memcpy(row, first->data, sizeof(struct db_row)); + + db_list_free(table); + + return row; }
void CSQLite3::SetSettings(const map_plugin_settings_t& pSettings) diff --git a/lib/plugins/SQLite3.h b/lib/plugins/SQLite3.h index d2af864..3dfd0a9 100644 --- a/lib/plugins/SQLite3.h +++ b/lib/plugins/SQLite3.h @@ -47,10 +47,11 @@ class CSQLite3 : public CDatabase virtual void SetReportedPerReporter(const char *crash_id, const char *reporter, const char *pMessage); - virtual vector_database_rows_t GetUIDData(long caller_uid); - virtual database_row_t GetRow(const char *crash_id); + virtual GList *GetUIDData(long caller_uid); + virtual struct db_row *GetRow(const char *crash_id);
virtual void SetSettings(const map_plugin_settings_t& pSettings); };
+ #endif diff --git a/lib/utils/Makefile.am b/lib/utils/Makefile.am index cc77c82..15fd03e 100644 --- a/lib/utils/Makefile.am +++ b/lib/utils/Makefile.am @@ -31,7 +31,8 @@ libABRTUtils_la_SOURCES = \ backtrace_parser.y \ strbuf.c strbuf.h \ abrt_packages.c abrt_packages.h \ - hooklib.c hooklib.h + hooklib.c hooklib.h \ + database.c libABRTUtils_la_CPPFLAGS = \ -Wall -Werror \ -I$(srcdir)/../../inc \ diff --git a/lib/utils/database.c b/lib/utils/database.c new file mode 100644 index 0000000..f572a5b --- /dev/null +++ b/lib/utils/database.c @@ -0,0 +1,52 @@ +/* + Copyright (C) 2010 ABRT team + Copyright (C) 2010 RedHat Inc + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License along + with this program; if not, write to the Free Software Foundation, Inc., + 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ + +#include "abrtlib.h" +#include "database.h" + +void db_row_free(struct db_row *row) +{ + if (!row) + return; + + free(row->db_uuid); + free(row->db_uid); + free(row->db_inform_all); + free(row->db_dump_dir); + free(row->db_count); + free(row->db_reported); + free(row->db_message); + free(row->db_time); + + free(row); +} + +void db_list_free(GList *list) +{ + if (!list) + return; + + for (GList *li = list; li != NULL; li = g_list_next(li)) + { + struct db_row *row = (struct db_row*)li->data; + db_row_free(row); + } + g_list_free(list); +} + diff --git a/src/daemon/CrashWatcher.cpp b/src/daemon/CrashWatcher.cpp index 6310118..416d5f4 100644 --- a/src/daemon/CrashWatcher.cpp +++ b/src/daemon/CrashWatcher.cpp @@ -188,27 +188,33 @@ int DeleteDebugDump(const char *crash_id, long caller_uid) { CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - database_row_t row = database->GetRow(crash_id); - if (row.m_sUUID == "") + struct db_row *row = database->GetRow(crash_id); + if (!row) { database->DisConnect(); return ENOENT; } + + char caller_uid_str[sizeof(long) * 3 + 2]; + sprintf(caller_uid_str, "%li", caller_uid); + if (caller_uid != 0 /* not called by root */ - && row.m_sInformAll != "1" - && to_string(caller_uid) != row.m_sUID + && row->db_inform_all[0] != '1' + && strcmp(caller_uid_str, row->db_uid) != 0 ) { database->DisConnect(); + db_row_free(row); return EPERM; } database->DeleteRow(crash_id); database->DisConnect(); - const char *dump_dir = row.m_sDebugDumpDir.c_str(); - if (dump_dir[0] != '\0') + if (row->db_dump_dir[0] != '\0') { - delete_debug_dump_dir(dump_dir); + delete_debug_dump_dir(row->db_dump_dir); + db_row_free(row); return 0; /* success */ } + db_row_free(row); } catch (CABRTException& e) { diff --git a/src/daemon/MiddleWare.cpp b/src/daemon/MiddleWare.cpp index 7752dc3..9f0f815 100644 --- a/src/daemon/MiddleWare.cpp +++ b/src/daemon/MiddleWare.cpp @@ -278,21 +278,26 @@ mw_result_t CreateCrashReport(const char *crash_id, { VERB2 log("CreateCrashReport('%s',%ld,result)", crash_id, caller_uid);
- database_row_t row; CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - row = database->GetRow(crash_id); + + struct db_row *row = database->GetRow(crash_id); database->DisConnect(); - if (row.m_sUUID == "") + if (!row) { error_msg("crash '%s' is not in database", crash_id); return MW_IN_DB_ERROR; } + + char caller_uid_str[sizeof(long) * 3 + 2]; + sprintf(caller_uid_str, "%li", caller_uid); + if (caller_uid != 0 /* not called by root */ - && row.m_sInformAll != "1" - && to_string(caller_uid) != row.m_sUID + && row->db_inform_all[0] != '1' + && strcmp(caller_uid_str, row->db_uid) != 0 ) { error_msg("crash '%s' can't be accessed by user with uid %ld", crash_id, caller_uid); + db_row_free(row); return MW_IN_DB_ERROR; }
@@ -300,9 +305,10 @@ mw_result_t CreateCrashReport(const char *crash_id, try { dump_dir_t *dd = dd_init(); - if (!dd_opendir(dd, row.m_sDebugDumpDir.c_str())) + if (!dd_opendir(dd, row->db_dump_dir)) { dd_close(dd); + db_row_free(row); return MW_ERROR; }
@@ -319,24 +325,26 @@ mw_result_t CreateCrashReport(const char *crash_id, // Why do we reload dump dir's data via DebugDumpToCrashReport?
VERB3 log(" run_analyser_CreateReport('%s')", analyzer.c_str()); - run_analyser_CreateReport(analyzer.c_str(), row.m_sDebugDumpDir.c_str(), force); + run_analyser_CreateReport(analyzer.c_str(), row->db_dump_dir, force);
- std::string dup_hash = GetGlobalUUID(analyzer.c_str(), row.m_sDebugDumpDir.c_str()); + std::string dup_hash = GetGlobalUUID(analyzer.c_str(), row->db_dump_dir); VERB3 log(" DUPHASH:'%s'", dup_hash.c_str());
- VERB3 log(" RunAnalyzerActions('%s','%s','%s',force=%d)", analyzer.c_str(), package_name, row.m_sDebugDumpDir.c_str(), force); - RunAnalyzerActions(analyzer.c_str(), package_name, row.m_sDebugDumpDir.c_str(), force); + VERB3 log(" RunAnalyzerActions('%s','%s','%s',force=%d)", analyzer.c_str(), package_name, row->db_dump_dir, force); + RunAnalyzerActions(analyzer.c_str(), package_name, row->db_dump_dir, force); free(package_name); - if (DebugDumpToCrashReport(row.m_sDebugDumpDir.c_str(), pCrashData)) + if (DebugDumpToCrashReport(row->db_dump_dir, pCrashData)) { - add_to_crash_data_ext(pCrashData, CD_UUID , CD_SYS, CD_ISNOTEDITABLE, row.m_sUUID.c_str()); + add_to_crash_data_ext(pCrashData, CD_UUID , CD_SYS, CD_ISNOTEDITABLE, row->db_uuid); add_to_crash_data_ext(pCrashData, CD_DUPHASH, CD_TXT, CD_ISNOTEDITABLE, dup_hash.c_str()); } else { + db_row_free(row); error_msg("Error loading crash data"); return MW_ERROR; } + db_row_free(row); } catch (CABRTException& e) { @@ -611,20 +619,23 @@ static bool is_debug_dump_saved(long uid, const char *debug_dump_dir)
CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - vector_database_rows_t rows = database->GetUIDData(uid); + GList *table = database->GetUIDData(uid); database->DisConnect();
- size_t ii; bool found = false; - for (ii = 0; ii < rows.size(); ii++) + struct db_row *row = NULL; + for (GList *li = table; li != NULL; li = g_list_next(li)) { - if (0 == strcmp(rows[ii].m_sDebugDumpDir.c_str(), debug_dump_dir)) + row = (struct db_row*)li->data; + if (0 == strcmp(row->db_dump_dir, debug_dump_dir)) { found = true; break; } }
+ db_list_free(table); + return found; }
@@ -1012,24 +1023,28 @@ static mw_result_t SaveDebugDumpToDatabase(const char *crash_id, database->Connect(); /* note: if [UUID,UID] record exists, pDebugDumpDir is not updated in the record */ database->Insert_or_Update(crash_id, inform_all_users, pDebugDumpDir, pTime); - database_row_t row = database->GetRow(crash_id); + + struct db_row *row = database->GetRow(crash_id); database->DisConnect();
mw_result_t res = FillCrashInfo(crash_id, pCrashData); if (res == MW_OK) { const char *first = get_crash_data_item_content(pCrashData, CD_DUMPDIR).c_str(); - if (row.m_sReported == "1") + if (row && row->db_reported[0] == '1') { log("Crash is in database already (dup of %s) and is reported", first); + db_row_free(row); return MW_REPORTED; } - if (row.m_sCount != "1") + if (row && xatou(row->db_count) > 1) { + db_row_free(row); log("Crash is in database already (dup of %s)", first); return MW_OCCURRED; } } + db_row_free(row); return res; }
@@ -1122,7 +1137,11 @@ mw_result_t FillCrashInfo(const char *crash_id, { CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); database->Connect(); - database_row_t row = database->GetRow(crash_id); + + struct db_row *row = database->GetRow(crash_id); + if (!row) + return MW_ERROR; + database->DisConnect();
std::string package; @@ -1131,23 +1150,26 @@ mw_result_t FillCrashInfo(const char *crash_id, std::string analyzer;
dump_dir_t *dd = dd_init(); - if (!dd_opendir(dd, row.m_sDebugDumpDir.c_str())) + if (!dd_opendir(dd, row->db_dump_dir)) { dd_close(dd); + db_row_free(row); return MW_ERROR; }
load_crash_data_from_debug_dump(dd, pCrashData); dd_close(dd);
- add_to_crash_data(pCrashData, CD_UID , row.m_sUID.c_str() ); - add_to_crash_data(pCrashData, CD_UUID , row.m_sUUID.c_str() ); - add_to_crash_data(pCrashData, CD_INFORMALL , row.m_sInformAll.c_str() ); - add_to_crash_data(pCrashData, CD_COUNT , row.m_sCount.c_str() ); - add_to_crash_data(pCrashData, CD_REPORTED , row.m_sReported.c_str() ); - add_to_crash_data(pCrashData, CD_MESSAGE , row.m_sMessage.c_str() ); - add_to_crash_data(pCrashData, CD_DUMPDIR , row.m_sDebugDumpDir.c_str()); - add_to_crash_data(pCrashData, FILENAME_TIME , row.m_sTime.c_str() ); + add_to_crash_data(pCrashData, CD_UID , row->db_uid ); + add_to_crash_data(pCrashData, CD_UUID , row->db_uuid ); + add_to_crash_data(pCrashData, CD_INFORMALL , row->db_inform_all ); + add_to_crash_data(pCrashData, CD_COUNT , row->db_count ); + add_to_crash_data(pCrashData, CD_REPORTED , row->db_reported ); + add_to_crash_data(pCrashData, CD_MESSAGE , row->db_message ); + add_to_crash_data(pCrashData, CD_DUMPDIR , row->db_dump_dir ); + add_to_crash_data(pCrashData, FILENAME_TIME , row->db_time ); + + db_row_free(row);
return MW_OK; } @@ -1155,17 +1177,20 @@ mw_result_t FillCrashInfo(const char *crash_id, void GetUUIDsOfCrash(long caller_uid, vector_string_t &result) { CDatabase* database = g_pPluginManager->GetDatabase(g_settings_sDatabase.c_str()); - vector_database_rows_t rows; database->Connect(); - rows = database->GetUIDData(caller_uid); + GList *rows = database->GetUIDData(caller_uid); database->DisConnect();
- unsigned ii; - for (ii = 0; ii < rows.size(); ii++) + struct db_row *row = NULL; + for (GList *li = rows; li != NULL; li = g_list_next(li)) { - string crash_id = ssprintf("%s:%s", rows[ii].m_sUID.c_str(), rows[ii].m_sUUID.c_str()); + row = (struct db_row*)li->data; + string crash_id = ssprintf("%s:%s", row->db_uid, row->db_uuid); result.push_back(crash_id); } + + // TODO: return GList + db_list_free(rows); }
void AddAnalyzerActionOrReporter(const char *pAnalyzer,
On Wed, 2010-09-08 at 15:44 +0200, Nikola Pajkovsky wrote:
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com
configure.ac | 2 +- inc/abrtlib.h | 3 + inc/database.h | 51 +++++++++++------- lib/plugins/Makefile.am | 4 +- lib/plugins/SQLite3.cpp | 127 +++++++++++++++++++++++++------------------ lib/plugins/SQLite3.h | 5 +- lib/utils/Makefile.am | 3 +- lib/utils/database.c | 52 ++++++++++++++++++ src/daemon/CrashWatcher.cpp | 20 +++++--- src/daemon/MiddleWare.cpp | 93 ++++++++++++++++++++------------ 10 files changed, 242 insertions(+), 118 deletions(-) create mode 100644 lib/utils/database.c
Looks ok to me.
On 09/08/2010 03:50 PM, Denys Vlasenko wrote:
On Wed, 2010-09-08 at 15:44 +0200, Nikola Pajkovsky wrote:
Signed-off-by: Nikola Pajkovskynpajkovs@redhat.com
configure.ac | 2 +- inc/abrtlib.h | 3 + inc/database.h | 51 +++++++++++------- lib/plugins/Makefile.am | 4 +- lib/plugins/SQLite3.cpp | 127 +++++++++++++++++++++++++------------------ lib/plugins/SQLite3.h | 5 +- lib/utils/Makefile.am | 3 +- lib/utils/database.c | 52 ++++++++++++++++++ src/daemon/CrashWatcher.cpp | 20 +++++--- src/daemon/MiddleWare.cpp | 93 ++++++++++++++++++++------------ 10 files changed, 242 insertions(+), 118 deletions(-) create mode 100644 lib/utils/database.c
Looks ok to me.
Thanks!
crash-catcher@lists.fedorahosted.org