On 04/18/2013 04:58 PM, Denys Vlasenko wrote:
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@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 :)
- I agree with Denys here, it's too confusing to show 2 reasons to users...
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);
+}