On 04/18/2013 03:03 PM, Jakub Filak wrote:
If count item is missing in dump directory at abrtd start, then
abrtd
considers that dump directory as unprocessed.
Closes #562
Signed-off-by: Jakub Filak <jfilak(a)redhat.com>
---
src/daemon/abrtd.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 111 insertions(+), 14 deletions(-)
diff --git a/src/daemon/abrtd.c b/src/daemon/abrtd.c
index 7001079..044eb41 100644
--- a/src/daemon/abrtd.c
+++ b/src/daemon/abrtd.c
@@ -892,6 +892,98 @@ static void start_syslog_logging()
putenv((char*)"ABRT_SYSLOG=1");
}
+/* The function expects that FILENAME_COUNT dump dir element is created by
+ * abrtd after all post-create events are successfully done. Thus if
+ * FILENAME_COUNT element doesn't exist abrtd can consider the dump directory
+ * as unprocessed.
+ *
+ * Relying on content of dump directory has one problem. If a hook provides
+ * FILENAME_COUNT abrtd will consider the dump directory as processed.
+ */
+static void mark_unprocessed_dump_dirs_not_reportable(const char *path)
+{
+ log("Searching for unprocessed dump directories");
+
+ DIR *dp = opendir(path);
+ if (!dp)
+ {
+ perror_msg("Can't find unprocessed dump directories in directory
'%s'", path);
I would say "Can't open directory '%s'".
+ return;
+ }
+
+ struct dirent *dent;
+ while ((dent = readdir(dp)) != NULL)
+ {
+ if (dot_or_dotdot(dent->d_name))
+ continue; /* skip "." and ".." */
+
+ char *full_name = concat_path_file(path, dent->d_name);
+
+ struct stat stat_buf;
+ if (stat(full_name, &stat_buf) != 0)
+ {
+ log("Can't access path '%s'", full_name);
this should be perror_msg(), not log().
+ goto next_dd;
+ }
+
+ if (S_ISDIR(stat_buf.st_mode) == 0)
+ /* This is expected. The dump location contains some aux files */
+ goto next_dd;
+
+ struct dump_dir *dd = dd_opendir(full_name, /*flags*/0);
+ if (dd)
+ {
+ if (!dd_exist(dd, FILENAME_COUNT))
+ {
+ log("Marking dump dir '%s' as not reportable because of
missing '"FILENAME_COUNT"' item",
+ full_name);
"dump dir" -> "problem dir[ectory]", or remove this part
altogether:
"Marking /var/tmp/cccp-FOO-BAR not reportable..."
already looks understandable without those words.
Can shorten this part: change
"... because of missing '"FILENAME_COUNT"' item"
to
"... (no '"FILENAME_COUNT"' item)"
+
+ const char *reason = _("The problem data are incomplete. This
"
+ "usually happens when a problem is detected while
"
+ "computer is shutting down or user is logging out.
"
+ "In order to provide valuable problem reports, ABRT
"
+ "will not allow you to submit this problem. If you
"
+ "have time and want to help the developers in their
"
+ "effort to sort out this problem, please contact
"
+ "them directly.");
+
+ /* Add dummy count 0 (valid dump directories has count >= 1)
+ * Doing it before updating FILENAME_NOT_REPORTABLE item in
+ * order to avoid unexpected growing of FILENAME_NOT_REPORTABLE
+ * item. And if any of the following operation will cause a
+ * failure of abrtd, then abrtd will not go there again after
+ * restart.
+ */
+ VERB1 log("Adding dummy count '0' to '%s'",
full_name);
+ dd_save_text(dd, FILENAME_COUNT, "0");
+
+ char *old_not_reportable = dd_load_text_ext(dd,
FILENAME_NOT_REPORTABLE,
+ DD_FAIL_QUIETLY_ENOENT | DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);
+ if (old_not_reportable)
+ { /* Add the reason to the beginnig of
+ * FILENAME_NOT_REPORTABLE item because this has higher
+ * priority (two new lines because the reason doesn't
+ * have any)
+ */
Hmmm.
Maybe if not_reportable already exists, just skip the directory altogether?
This eliminates the risk of increasing confusion :)
+ char *message = xasprintf("%s\n\n%s",
reason, old_not_reportable);
+ dd_save_text(dd, FILENAME_NOT_REPORTABLE, message);
+ free(message);
+ free(old_not_reportable);
+ }
+ else
+ dd_save_text(dd, FILENAME_NOT_REPORTABLE, reason);
+ }
+ dd_close(dd);
+ }
+ else
+ log("Found an invalid dump directory '%s'", full_name);
Wouldn't dd_opendir() already emit a message explaining what's wrong?
+
+ next_dd:
+ free(full_name);
+ }
+ closedir(dp);
+}
--
vda