DD_CLOSE_ON_OPEN_ERR flag indicate to free all memory allocated by
dd_init() which is passed to dd_opendir().
dd_opendir fails when dump_dir does not exist.
VERB1 log(_("Unable to open debug dump '%s'"), pDebugDumpDir);
is all removed because there is error_msg("'%s' does not exist",
dd->dd_dir); in dd_opendir().
There are two places where condition was inverted.
Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com>
---
inc/dump_dir.h | 6 ++-
lib/plugins/CCpp.cpp | 28 ++-------
lib/plugins/Kerneloops.cpp | 6 +--
lib/plugins/Python.cpp | 6 +--
lib/plugins/RunApp.cpp | 6 +--
lib/plugins/SOSreport.cpp | 6 +--
lib/utils/dump_dir.c | 21 ++-----
src/daemon/MiddleWare.cpp | 88 +++++++++++---------------
src/daemon/abrt-action-generate-backtrace.c | 13 +---
9 files changed, 60 insertions(+), 120 deletions(-)
diff --git a/inc/dump_dir.h b/inc/dump_dir.h
index 8e65b1f..f02e1a5 100644
--- a/inc/dump_dir.h
+++ b/inc/dump_dir.h
@@ -26,6 +26,10 @@
extern "C" {
#endif
+enum {
+ DD_CLOSE_ON_OPEN_ERR = 1 << 0,
+};
+
struct dump_dir {
char *dd_dir;
DIR *next_dir;
@@ -37,7 +41,7 @@ struct dump_dir {
struct dump_dir *dd_init(void);
void dd_close(struct dump_dir *dd);
-int dd_opendir(struct dump_dir *dd, const char *dir);
+int dd_opendir(struct dump_dir *dd, const char *dir, int flags);
int dd_exist(struct dump_dir *dd, const char *path);
int dd_create(struct dump_dir *dd, const char *dir, uid_t uid);
DIR *dd_init_next_file(struct dump_dir *dd);
diff --git a/lib/plugins/CCpp.cpp b/lib/plugins/CCpp.cpp
index c4a1f08..041c85b 100644
--- a/lib/plugins/CCpp.cpp
+++ b/lib/plugins/CCpp.cpp
@@ -209,12 +209,8 @@ static void GetIndependentBuildIdPC(const char *unstrip_n_output,
static char* run_unstrip_n(const char *pDebugDumpDir, unsigned timeout_sec)
{
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, pDebugDumpDir))
- {
- dd_close(dd);
- VERB1 log(_("Unable to open debug dump '%s'"), pDebugDumpDir);
+ if (!dd_opendir(dd, pDebugDumpDir, DD_CLOSE_ON_OPEN_ERR))
return NULL;
- }
char *uid = dd_load_text(dd, CD_UID);
dd_close(dd);
@@ -403,12 +399,8 @@ static void trim_debuginfo_cache(unsigned max_mb)
string CAnalyzerCCpp::GetLocalUUID(const char *pDebugDumpDir)
{
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, pDebugDumpDir))
- {
- dd_close(dd);
- VERB1 log(_("Unable to open debug dump '%s'"), pDebugDumpDir);
+ if (!dd_opendir(dd, pDebugDumpDir, DD_CLOSE_ON_OPEN_ERR))
return string("");
- }
char *executable = dd_load_text(dd, FILENAME_EXECUTABLE);
char *package = dd_load_text(dd, FILENAME_PACKAGE);
@@ -464,12 +456,8 @@ string CAnalyzerCCpp::GetLocalUUID(const char *pDebugDumpDir)
string CAnalyzerCCpp::GetGlobalUUID(const char *pDebugDumpDir)
{
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, pDebugDumpDir))
- {
- dd_close(dd);
- VERB1 log(_("Unable to open debug dump '%s'"), pDebugDumpDir);
+ if (!dd_opendir(dd, pDebugDumpDir, DD_CLOSE_ON_OPEN_ERR))
return string("");
- }
if (dd_exist(dd, FILENAME_GLOBAL_UUID))
{
@@ -640,12 +628,8 @@ void CAnalyzerCCpp::CreateReport(const char *pDebugDumpDir, int
force)
}
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, pDebugDumpDir))
- {
- dd_close(dd);
- VERB1 log(_("Unable to open debug dump '%s'"), pDebugDumpDir);
+ if (!dd_opendir(dd, pDebugDumpDir, DD_CLOSE_ON_OPEN_ERR))
return;
- }
if (!force)
{
@@ -688,11 +672,9 @@ void CAnalyzerCCpp::CreateReport(const char *pDebugDumpDir, int
force)
gen_backtrace(pDebugDumpDir, m_sDebugInfoDirs.c_str(), m_nGdbTimeoutSec);
dd = dd_init();
- if (!dd_opendir(dd, pDebugDumpDir))
+ if (!dd_opendir(dd, pDebugDumpDir, DD_CLOSE_ON_OPEN_ERR))
{
- dd_close(dd);
free(build_ids);
- VERB1 log(_("Unable to open debug dump '%s'"), pDebugDumpDir);
return;
}
diff --git a/lib/plugins/Kerneloops.cpp b/lib/plugins/Kerneloops.cpp
index 0c4b1a5..d3ce2e8 100644
--- a/lib/plugins/Kerneloops.cpp
+++ b/lib/plugins/Kerneloops.cpp
@@ -126,12 +126,8 @@ std::string CAnalyzerKerneloops::GetLocalUUID(const char
*pDebugDumpDir)
VERB3 log("Getting local universal unique identification");
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, pDebugDumpDir))
- {
- dd_close(dd);
- VERB1 log(_("Unable to open debug dump '%s'"), pDebugDumpDir);
+ if (!dd_opendir(dd, pDebugDumpDir, DD_CLOSE_ON_OPEN_ERR))
return std::string("");
- }
char *oops = dd_load_text(dd, FILENAME_BACKTRACE);
unsigned hash = hash_oops_str(oops);
diff --git a/lib/plugins/Python.cpp b/lib/plugins/Python.cpp
index 9ef830e..5e4cc02 100644
--- a/lib/plugins/Python.cpp
+++ b/lib/plugins/Python.cpp
@@ -26,12 +26,8 @@ using namespace std;
string CAnalyzerPython::GetLocalUUID(const char *pDebugDumpDir)
{
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, pDebugDumpDir))
- {
- dd_close(dd);
- VERB1 log(_("Unable to open debug dump '%s'"), pDebugDumpDir);
+ if (!dd_opendir(dd, pDebugDumpDir, DD_CLOSE_ON_OPEN_ERR))
return string("");
- }
char *bt = dd_load_text(dd, FILENAME_BACKTRACE);
dd_close(dd);
diff --git a/lib/plugins/RunApp.cpp b/lib/plugins/RunApp.cpp
index 0bf5ff0..3ae6638 100644
--- a/lib/plugins/RunApp.cpp
+++ b/lib/plugins/RunApp.cpp
@@ -58,12 +58,8 @@ void CActionRunApp::Run(const char *pActionDir, const char *pArgs, int
force)
if (args.size() > FILENAME)
{
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, pActionDir))
- {
- dd_close(dd);
- VERB1 log(_("Unable to open debug dump '%s'"),
pActionDir);
+ if (!dd_opendir(dd, pActionDir, DD_CLOSE_ON_OPEN_ERR))
return;
- }
dd_save_binary(dd, args[FILENAME].c_str(), cmd_out, cmd_out_size);
dd_close(dd);
diff --git a/lib/plugins/SOSreport.cpp b/lib/plugins/SOSreport.cpp
index 280738d..5c1b658 100644
--- a/lib/plugins/SOSreport.cpp
+++ b/lib/plugins/SOSreport.cpp
@@ -51,12 +51,8 @@ void CActionSOSreport::Run(const char *pActionDir, const char *pArgs,
int force)
if (!force)
{
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, pActionDir))
- {
- dd_close(dd);
- VERB1 log(_("Unable to open debug dump '%s'"),
pActionDir);
+ if (!dd_opendir(dd, pActionDir, DD_CLOSE_ON_OPEN_ERR))
return;
- }
bool bt_exists = dd_exist(dd, "sosreport.tar.bz2") || dd_exist(dd,
"sosreport.tar.xz");
if (bt_exists)
diff --git a/lib/utils/dump_dir.c b/lib/utils/dump_dir.c
index 2602ede..f81e7c9 100644
--- a/lib/utils/dump_dir.c
+++ b/lib/utils/dump_dir.c
@@ -25,16 +25,6 @@
// TODO:
//
-// dd_opendir needs a bit flags parameter, with bits for
-// "auto-close on error" and "log error message on error".
-// This will simplify a ton of places which do this:
-// if (!dd_opendir(dd, dirname))
-// {
-// dd_close(dd);
-// VERB1 log(_("Unable to open debug dump '%s'"), dirname);
-// return ........;
-// }
-//
// Perhaps dd_opendir should do some sanity checking like
// "if there is no "uid" file in the directory, it's not a crash
dump",
// and fail.
@@ -99,7 +89,7 @@ void dd_close(struct dump_dir *dd)
free(dd);
}
-int dd_opendir(struct dump_dir *dd, const char *dir)
+int dd_opendir(struct dump_dir *dd, const char *dir, int flags)
{
if (dd->locked)
error_msg_and_die("dump_dir is already opened"); /* bug */
@@ -108,7 +98,9 @@ int dd_opendir(struct dump_dir *dd, const char *dir)
if (!exist_file_dir(dd->dd_dir))
{
error_msg("'%s' does not exist", dd->dd_dir);
- free(dd->dd_dir);
+ if (flags & DD_CLOSE_ON_OPEN_ERR)
+ dd_close(dd);
+
return 0;
}
@@ -485,9 +477,8 @@ int dd_get_next_file(struct dump_dir *dd, char **short_name, char
**full_name)
void delete_debug_dump_dir(const char *dd_dir)
{
struct dump_dir *dd = dd_init();
- if (dd_opendir(dd, dd_dir))
+ if (dd_opendir(dd, dd_dir, 0))
dd_delete(dd);
- else
- VERB1 log("Unable to open debug dump '%s'", dd_dir);
+
dd_close(dd);
}
diff --git a/src/daemon/MiddleWare.cpp b/src/daemon/MiddleWare.cpp
index ee35369..bfab9f0 100644
--- a/src/daemon/MiddleWare.cpp
+++ b/src/daemon/MiddleWare.cpp
@@ -189,28 +189,25 @@ static bool DebugDumpToCrashReport(const char *pDebugDumpDir,
map_crash_data_t&
VERB3 log(" DebugDumpToCrashReport('%s')", pDebugDumpDir);
struct dump_dir *dd = dd_init();
- if (dd_opendir(dd, pDebugDumpDir))
+ if (!dd_opendir(dd, pDebugDumpDir, DD_CLOSE_ON_OPEN_ERR))
+ return false;
+
+ const char *const *v = must_have_files;
+ while (*v)
{
- const char *const *v = must_have_files;
- while (*v)
+ if (!dd_exist(dd, *v))
{
- if (!dd_exist(dd, *v))
- {
- dd_close(dd);
- throw CABRTException(EXCEP_ERROR, "DebugDumpToCrashReport():
important file '%s' is missing", *v);
- }
-
- v++;
+ dd_close(dd);
+ throw CABRTException(EXCEP_ERROR, "DebugDumpToCrashReport(): important
file '%s' is missing", *v);
}
- load_crash_data_from_debug_dump(dd, pCrashData);
- dd_close(dd);
-
- return true;
+ v++;
}
+ load_crash_data_from_debug_dump(dd, pCrashData);
dd_close(dd);
- return false;
+
+ return true;
}
/**
@@ -305,9 +302,8 @@ mw_result_t CreateCrashReport(const char *crash_id,
try
{
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, row->db_dump_dir))
+ if (!dd_opendir(dd, row->db_dump_dir, DD_CLOSE_ON_OPEN_ERR))
{
- dd_close(dd);
db_row_free(row);
return MW_ERROR;
}
@@ -462,7 +458,7 @@ report_status_t Report(const map_crash_data_t& client_report,
if (comment || reproduce || backtrace)
{
struct dump_dir *dd = dd_init();
- if (dd_opendir(dd, pDumpDir.c_str()))
+ if (dd_opendir(dd, pDumpDir.c_str(), 0))
{
if (comment)
{
@@ -738,11 +734,8 @@ static mw_result_t SavePackageDescriptionToDebugDump(
VERB2 log("Crash in unpackaged executable '%s', proceeding
without packaging information", pExecutable);
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, pDebugDumpDir))
- {
- dd_close(dd);
+ if (!dd_opendir(dd, pDebugDumpDir, DD_CLOSE_ON_OPEN_ERR))
return MW_ERROR;
- }
dd_save_text(dd, FILENAME_PACKAGE, "");
dd_save_text(dd, FILENAME_COMPONENT, "");
@@ -863,35 +856,32 @@ static mw_result_t SavePackageDescriptionToDebugDump(
}
struct dump_dir *dd = dd_init();
- if (dd_opendir(dd, pDebugDumpDir))
+ if (!dd_opendir(dd, pDebugDumpDir, DD_CLOSE_ON_OPEN_ERR))
+ return MW_ERROR;
+
+ if (rpm_pkg)
{
- if (rpm_pkg)
- {
- dd_save_text(dd, FILENAME_PACKAGE, rpm_pkg);
- free(rpm_pkg);
- }
+ dd_save_text(dd, FILENAME_PACKAGE, rpm_pkg);
+ free(rpm_pkg);
+ }
- if (dsc)
- {
- dd_save_text(dd, FILENAME_DESCRIPTION, dsc);
- free(dsc);
- }
+ if (dsc)
+ {
+ dd_save_text(dd, FILENAME_DESCRIPTION, dsc);
+ free(dsc);
+ }
- if (component)
- {
- dd_save_text(dd, FILENAME_COMPONENT, component);
- free(component);
- }
+ if (component)
+ {
+ dd_save_text(dd, FILENAME_COMPONENT, component);
+ free(component);
+ }
- if (!remote)
- dd_save_text(dd, FILENAME_HOSTNAME, host);
+ if (!remote)
+ dd_save_text(dd, FILENAME_HOSTNAME, host);
- dd_close(dd);
- return MW_OK;
- }
dd_close(dd);
-
- return MW_ERROR;
+ return MW_OK;
}
bool analyzer_has_InformAllUsers(const char *analyzer_name)
@@ -1054,11 +1044,8 @@ mw_result_t SaveDebugDump(const char *pDebugDumpDir,
int remote = 0;
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, pDebugDumpDir))
- {
- dd_close(dd);
+ if (!dd_opendir(dd, pDebugDumpDir, DD_CLOSE_ON_OPEN_ERR))
return MW_ERROR;
- }
char *time = dd_load_text(dd, FILENAME_TIME);
char *uid = dd_load_text(dd, CD_UID);
@@ -1143,9 +1130,8 @@ mw_result_t FillCrashInfo(const char *crash_id,
return MW_ERROR;
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, row->db_dump_dir))
+ if (!dd_opendir(dd, row->db_dump_dir, DD_CLOSE_ON_OPEN_ERR))
{
- dd_close(dd);
db_row_free(row);
return MW_ERROR;
}
diff --git a/src/daemon/abrt-action-generate-backtrace.c
b/src/daemon/abrt-action-generate-backtrace.c
index 8bedf54..46f002a 100644
--- a/src/daemon/abrt-action-generate-backtrace.c
+++ b/src/daemon/abrt-action-generate-backtrace.c
@@ -303,12 +303,8 @@ int main(int argc, char **argv)
}
struct dump_dir *dd = dd_init();
- if (!dd_opendir(dd, dump_dir_name))
- {
- dd_close(dd);
- VERB1 log(_("Unable to open debug dump '%s'"), dump_dir_name);
+ if (!dd_opendir(dd, dump_dir_name, DD_CLOSE_ON_OPEN_ERR))
return 1;
- }
char *package = dd_load_text(dd, FILENAME_PACKAGE);
char *executable = dd_load_text(dd, FILENAME_EXECUTABLE);
@@ -323,12 +319,9 @@ int main(int argc, char **argv)
}
dd = dd_init();
- if (!dd_opendir(dd, dump_dir_name))
- {
- dd_close(dd);
- VERB1 log(_("Unable to open debug dump '%s'"), dump_dir_name);
+ if (!dd_opendir(dd, dump_dir_name, DD_CLOSE_ON_OPEN_ERR))
return 1;
- }
+
dd_save_text(dd, FILENAME_BACKTRACE, backtrace_str);
/* Compute and store backtrace hash. */
--
1.7.1