[ABRT PATCH] don't blame interpreter, blame the running script #609
by Jiri Moskovcak
---
src/daemon/abrt-action-save-package-data.c | 50 +++++++++++++++++++++------
src/daemon/abrt-action-save-package-data.conf | 3 ++
2 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/src/daemon/abrt-action-save-package-data.c b/src/daemon/abrt-action-save-package-data.c
index 8921b6c..8060d16 100644
--- a/src/daemon/abrt-action-save-package-data.c
+++ b/src/daemon/abrt-action-save-package-data.c
@@ -25,6 +25,7 @@ static GList *settings_setOpenGPGPublicKeys = NULL;
static GList *settings_setBlackListedPkgs = NULL;
static GList *settings_setBlackListedPaths = NULL;
static bool settings_bProcessUnpackaged = false;
+static GList *settings_Interpreters = NULL;
static GList *parse_list(const char* list)
{
@@ -88,6 +89,13 @@ static void ParseCommon(map_string_h *settings, const char *conf_filename)
g_hash_table_remove(settings, "ProcessUnpackaged");
}
+ value = g_hash_table_lookup(settings, "Interpreters");
+ if (value)
+ {
+ settings_Interpreters = parse_list(value);
+ g_hash_table_remove(settings, "Interpreters");
+ }
+
GHashTableIter iter;
char *name;
/*char *value; - already declared */
@@ -181,6 +189,17 @@ static bool is_path_blacklisted(const char *path)
return false;
}
+static bool is_string_in_list(GList *list, const char *string)
+{
+ GList *li;
+ for (li = list; li != NULL; li = g_list_next(li))
+ {
+ if (strcmp((char *)li->data, string) == 0)
+ return true;
+ }
+ return false;
+}
+
static struct pkg_envra *get_script_name(const char *cmdline, char **executable)
{
// TODO: we don't verify that python executable is not modified
@@ -282,12 +301,27 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name)
else
basename = executable;
- if (!strcmp(basename, "python")
- || !strcmp(basename, "perl"))
+ /* if basename is known interpreter, we want to blame the running script
+ * not the interpreter
+ */
+ if (is_string_in_list(settings_Interpreters, basename))
{
struct pkg_envra *script_pkg = get_script_name(cmdline, &executable);
if (!script_pkg)
- goto ret;
+ {
+ /* none or unknown script, and config says we don't care about
+ * unpackaged things
+ */
+ if (!settings_bProcessUnpackaged)
+ goto ret;
+
+ /* unpackaged script, but the settings says we want to keep it
+ * bz plugin wont allow to report this anyway, because component
+ * is missing, so there is no reason to mark it as not_reportable
+ * someone might want to use abrt to report it using ftp
+ */
+ goto ret0;
+ }
free_pkg_envra(pkg_name);
pkg_name = script_pkg;
@@ -296,15 +330,11 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name)
package_short_name = xasprintf("%s", pkg_name->p_name);
VERB2 log("Package:'%s' short:'%s'", pkg_name->p_nvr, package_short_name);
- GList *li;
- for (li = settings_setBlackListedPkgs; li != NULL; li = g_list_next(li))
+ if (is_string_in_list(settings_setBlackListedPkgs, package_short_name))
{
- if (strcmp((char*)li->data, package_short_name) == 0)
- {
- log("Blacklisted package '%s'", package_short_name);
- goto ret; /* return 1 (failure) */
- }
+ log("Blacklisted package '%s'", package_short_name);
+ goto ret; /* return 1 (failure) */
}
if (settings_bOpenGPGCheck)
diff --git a/src/daemon/abrt-action-save-package-data.conf b/src/daemon/abrt-action-save-package-data.conf
index 643259a..3d35bb6 100644
--- a/src/daemon/abrt-action-save-package-data.conf
+++ b/src/daemon/abrt-action-save-package-data.conf
@@ -16,3 +16,6 @@ ProcessUnpackaged = no
# Blacklisted executable paths (shell patterns)
#
BlackListedPaths = /usr/share/doc/*, */example*, /usr/bin/nspluginviewer, /usr/lib/xulrunner-*/plugin-container
+
+# interpreters names
+Interpreters = python2, python2.7, python, python3, python3.3, perl, perl5.16.2
--
1.8.1.2
11 years, 2 months
[LIBREPORT PATCH] ss: add a timeout to the wainting for the Completed signal
by Jakub Filak
- closes rhbz#901467
Signed-off-by: Jakub Filak <jfilak(a)redhat.com>
---
src/gtk-helpers/secrets.c | 132 +++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 118 insertions(+), 14 deletions(-)
diff --git a/src/gtk-helpers/secrets.c b/src/gtk-helpers/secrets.c
index 9de2284..c3c1f68 100644
--- a/src/gtk-helpers/secrets.c
+++ b/src/gtk-helpers/secrets.c
@@ -34,6 +34,9 @@
/* 5s timeout*/
#define SECRETS_CALL_DEFAULT_TIMEOUT 5000
+/* 15s until the timeout dialog will appear */
+#define PROMPT_TIMEOUT_SECONDS 15
+
/* Well known errors for which we have workarounds */
#define NOT_IMPLEMENTED_READ_ALIAS_ERROR "org.freedesktop.DBus.Error.UnknownMethod"
#define INVALID_PROPERTIES_ARGUMENTS_ERROR "org.freedesktop.DBus.Error.InvalidArgs"
@@ -377,24 +380,41 @@ static void secrets_service_set_unavailable(void)
/* http://standards.freedesktop.org/secret-service/re05.html */
/*******************************************************************************/
+/*
+ * Holds all data necessary for management of the glib main loop with the
+ * timeout dialog.
+ */
+struct secrets_loop_env
+{
+ GMainContext* gcontext;
+ GMainLoop *gloop;
+ GtkWidget *timeout_dialog;
+};
+
struct prompt_source
{
GSource source;
GDBusProxy *prompt_proxy;
- GMainLoop *loop;
+ struct secrets_loop_env *env;
};
struct prompt_call_back_args
{
gboolean dismissed;
GVariant *result;
- GMainLoop *loop;
+ struct secrets_loop_env *env;
};
-static void prompt_quit_loop(GMainLoop *loop)
+static void prompt_quit_loop(struct secrets_loop_env *env)
{
- if (g_main_loop_is_running(loop))
- g_main_loop_quit(loop);
+ if (NULL != env->timeout_dialog)
+ {
+ gtk_widget_destroy(env->timeout_dialog);
+ env->timeout_dialog = NULL;
+ }
+
+ if (g_main_loop_is_running(env->gloop))
+ g_main_loop_quit(env->gloop);
}
static gboolean prompt_g_idle_prepare(GSource *source_data, gint *timeout)
@@ -425,7 +445,7 @@ static gboolean prompt_g_idle_dispatch(GSource *source,
if (!resp)
/* We have to kill the loop because a dbus call failed and the signal */
/* which stops the loop won't be received */
- prompt_quit_loop(prompt_source->loop);
+ prompt_quit_loop(prompt_source->env);
else
g_variant_unref(resp);
@@ -454,7 +474,7 @@ static void prompt_call_back(GDBusConnection *connection, const gchar *sender_na
/* if the prompt or operation were dismissed we don't care about result */
g_variant_unref(result);
- prompt_quit_loop(args->loop);
+ prompt_quit_loop(args->env);
}
/*
@@ -467,6 +487,83 @@ static bool is_prompt_required(const char *prompt_path)
}
/*
+ * Sets the timeout dialog to be displayed after PROMPT_TIMEOUT_SECONDS.
+ *
+ * @param env Crate holding necessary values.
+ */
+static void prompt_timeout_start(struct secrets_loop_env *env);
+
+/*
+ * A response callback which stops main loop execution or starts a new timeout.
+ *
+ * The main loop is stopped if user replied with "YES"; otherwise a new timeout
+ * is started.
+ */
+static void timeout_dialog_response_cb(GtkDialog *dialog, gint response_id, gpointer user_data)
+{
+ struct secrets_loop_env *env = (struct secrets_loop_env *)user_data;
+
+ gtk_widget_hide(env->timeout_dialog);
+
+ /* YES means 'Yes I want to stop wainting for Secret Service' */
+ /* for other responses we have to start a new timeout */
+ if (GTK_RESPONSE_YES == response_id)
+ prompt_quit_loop(env);
+ else
+ prompt_timeout_start(env);
+}
+
+/*
+ * Shows the timeout dialog which should allow user to break infinite wainting
+ * for Completed signal emitted by the Secret Service
+ *
+ * @param user_data Should hold an pointer to struct secrets_loop_env
+ */
+static gboolean prompt_timeout_cb(gpointer user_data)
+{
+ VERB3 log("A timeout was reached while waiting for the DBus Secret Service to get prompt result.");
+ struct secrets_loop_env *env = (struct secrets_loop_env *)user_data;
+
+ if (env->timeout_dialog == NULL)
+ {
+ GtkWidget *dialog = gtk_message_dialog_new(/*parent widget*/NULL,
+ GTK_DIALOG_MODAL,
+ GTK_MESSAGE_WARNING,
+ GTK_BUTTONS_YES_NO,
+ _("A timeout was reached while waiting for a prompt result from the DBus Secret Service."));
+
+ gtk_message_dialog_format_secondary_text(GTK_MESSAGE_DIALOG(dialog),
+ _("Do you want to stop waiting and continue in reporting without properly loaded configuration?"));
+
+ g_signal_connect(dialog, "response", G_CALLBACK(timeout_dialog_response_cb), user_data);
+
+ env->timeout_dialog = dialog;
+ }
+
+ gtk_widget_show_all(env->timeout_dialog);
+
+ return FALSE; /* remove this source */
+}
+
+/*
+ * Sets the timeout dialog to be displayed after PROMPT_TIMEOUT_SECONDS.
+ *
+ * @param env Crate holding necessary values.
+ */
+static void prompt_timeout_start(struct secrets_loop_env *env)
+{
+ GSource *timeout_source= g_timeout_source_new_seconds(PROMPT_TIMEOUT_SECONDS);
+
+ g_source_set_callback(timeout_source, prompt_timeout_cb, /*callback arg*/env, /*arg destroyer*/NULL);
+ g_source_attach(timeout_source, env->gcontext);
+
+ /* remove local reference, the source will be destroyed by the GMainContext
+ * over taken from glib2/gmain.c: g_timeout_add_full()
+ */
+ g_source_unref(timeout_source);
+}
+
+/*
* Perform a prompt
*
* @param prompt_path An object path pointing to a prompt
@@ -496,12 +593,16 @@ static GVariant *secrets_prompt(const char *prompt_path,
/* We have to use the thread default main context because a dbus signal callback */
/* will be invoked in the thread default main loop */
GMainContext *context = g_main_context_get_thread_default();
- GMainLoop *loop = g_main_loop_new(context, FALSE);
+ struct secrets_loop_env env = {
+ .gcontext=context,
+ .gloop=g_main_loop_new(context, FALSE),
+ .timeout_dialog=NULL,
+ };
struct prompt_call_back_args args = {
.result=NULL,
.dismissed=FALSE,
- .loop=loop
+ .env=&env
};
/* g_dbus_connection_signal_subscribe() doesn't report any error */
@@ -517,22 +618,25 @@ static GVariant *secrets_prompt(const char *prompt_path,
/* Idle source simply performs a prompt after the loop is stared. */
struct prompt_source *const prompt_source =
(struct prompt_source*)g_source_new(&idle_funcs, sizeof(*prompt_source));
-
prompt_source->prompt_proxy = prompt_proxy;
+ prompt_source->env = &env;
g_source_attach((GSource*)prompt_source, context);
- /* the loop is exited when the Completed signal is received */
/* the loop may sucks in infinite loop if the signal is never received */
- /* TODO : if timeout is required use g_timeout_source_new() and g_source_attach() */
- g_main_loop_run(loop);
+ /* thus in order to prevent it we use this timeout */
+ prompt_timeout_start(&env);
+
+ /* the loop is exited when the Completed signal is received */
+ g_main_loop_run(env.gloop);
/* destroy prompt source */
g_object_unref(prompt_proxy);
g_source_destroy((GSource*)prompt_source);
+ g_source_unref((GSource*)prompt_source);
g_dbus_connection_signal_unsubscribe(g_connection, signal_ret);
- g_main_loop_unref(loop);
+ g_main_loop_unref(env.gloop);
*dismissed = args.dismissed;
return args.result;
--
1.8.1.2
11 years, 2 months
[ABRT PATCH] applet: pass problem's id to the gui app
by Jakub Filak
- realted to rhbz#905412
- blocked by https://github.com/abrt/gnome-abrt/issues/10
Signed-off-by: Jakub Filak <jfilak(a)redhat.com>
---
src/applet/applet.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/applet/applet.c b/src/applet/applet.c
index b629e81..3f3b65f 100644
--- a/src/applet/applet.c
+++ b/src/applet/applet.c
@@ -433,7 +433,7 @@ static void new_dir_exists(GList **new_dirs)
list_free_with_free(dirlist);
}
-static void fork_exec_gui(void)
+static void fork_exec_gui(const char *problem_id)
{
fflush(NULL); /* paranoia */
pid_t pid = fork();
@@ -456,12 +456,22 @@ static void fork_exec_gui(void)
_exit(0);
}
+ // pass s_[] as DIR param(s) to 'gui executable'
+ char *gui_args[4];
+ char **pp = gui_args;
+ *pp++ = (char *)GUI_EXECUTABLE;
+ if (problem_id != NULL)
+ {
+ *pp++ = (char *)"-p";
+ *pp++ = (char *)problem_id;
+ }
+ *pp = NULL;
+
/* grandchild */
- //TODO: pass s_dirs[] as DIR param(s) to 'gui executable'
- execl(BIN_DIR"/"GUI_EXECUTABLE, GUI_EXECUTABLE, (char*) NULL);
+ execv(BIN_DIR"/"GUI_EXECUTABLE, gui_args);
/* Did not find 'gui executable' in installation directory. Oh well */
/* Trying to find it in PATH */
- execlp(GUI_EXECUTABLE, GUI_EXECUTABLE, (char*) NULL);
+ execvp(GUI_EXECUTABLE, gui_args);
perror_msg_and_die(_("Can't execute '%s'"), GUI_EXECUTABLE);
}
@@ -604,7 +614,7 @@ static void action_known(NotifyNotification *notification, gchar *action, gpoint
}
else if (strcmp(A_KNOWN_OPEN_GUI, action) == 0)
/* TODO teach gui to select a problem passed on cmd line */
- fork_exec_gui();
+ fork_exec_gui(pi->problem_dir);
else
/* This should not happen; otherwise it's a bug */
error_msg("%s:%d %s(): BUG Unknown action '%s'", __FILE__, __LINE__, __func__, action);
@@ -749,7 +759,7 @@ static GtkWidget *create_menu(void)
static void on_applet_activate_cb(GtkStatusIcon *status_icon, gpointer user_data)
{
- fork_exec_gui();
+ fork_exec_gui(/* problem id */ NULL);
hide_icon();
}
--
1.8.1.2
11 years, 2 months
[ABRT PATCH] applet: don't parse dd's time on its own
by Jakub Filak
- related to https://github.com/abrt/libreport/issues/123
Signed-off-by: Jakub Filak <jfilak(a)redhat.com>
---
src/applet/applet.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/src/applet/applet.c b/src/applet/applet.c
index b629e81..7bf7420 100644
--- a/src/applet/applet.c
+++ b/src/applet/applet.c
@@ -1396,17 +1396,8 @@ int main(int argc, char** argv)
continue;
}
- char *time_str = dd_load_text_ext(dd, FILENAME_TIME,
- DD_FAIL_QUIETLY_ENOENT | DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);
-
- if (time_str == NULL)
- {
- error_msg(_("dd_open() opened problem '%s' without valid time element"), (char *)new_dirs->data);
- goto next;
- }
-
/* Don't check errors, time element is always valid time stamp!! */
- if (strtoul(time_str, 0, 10) < min_born_time)
+ if (dd->dd_time < min_born_time)
{
VERB1 log("Ignoring outdated problem '%s'", (char *)new_dirs->data);
goto next;
@@ -1439,7 +1430,6 @@ int main(int argc, char** argv)
free(reported_to);
next:
- free(time_str);
dd_close(dd);
new_dirs = g_list_next(new_dirs);
--
1.8.1.2
11 years, 2 months
[LIBREPORT PATCH] fixed the previous commit
by Jiri Moskovcak
---
src/include/dump_dir.h | 3 +++
src/lib/dump_dir.c | 1 +
2 files changed, 4 insertions(+)
diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h
index 354c9c1..0d27a29 100644
--- a/src/include/dump_dir.h
+++ b/src/include/dump_dir.h
@@ -119,6 +119,9 @@ void delete_dump_dir(const char *dirname);
*/
int dump_dir_accessible_by_uid(const char *dirname, uid_t uid);
+/* creates not_reportable file in the problem directory and saves the
+ reason to it, which prevents libreport from reporting the problem
+*/
int dd_mark_as_notreportable(struct dump_dir *dd, const char *reason);
#ifdef __cplusplus
diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
index fde9a9d..e6d6ba9 100644
--- a/src/lib/dump_dir.c
+++ b/src/lib/dump_dir.c
@@ -1249,4 +1249,5 @@ int dd_mark_as_notreportable(struct dump_dir *dd, const char *reason)
}
dd_save_text(dd, FILENAME_NOT_REPORTABLE, reason);
+ return 0;
}
--
1.8.1.2
11 years, 2 months
[LIBREPORT PATCH] added helper for marking problem as not reportable
by Jiri Moskovcak
---
src/include/dump_dir.h | 2 ++
src/lib/dump_dir.c | 5 +++++
2 files changed, 7 insertions(+)
diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h
index 21169c8..c1b2a6e 100644
--- a/src/include/dump_dir.h
+++ b/src/include/dump_dir.h
@@ -119,6 +119,8 @@ void delete_dump_dir(const char *dirname);
*/
int dump_dir_accessible_by_uid(const char *dirname, uid_t uid);
+void dd_mark_as_notreportable(struct dump_dir *dd, const char *reason);
+
#ifdef __cplusplus
}
#endif
diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
index f7732ea..99ba771 100644
--- a/src/lib/dump_dir.c
+++ b/src/lib/dump_dir.c
@@ -1239,3 +1239,8 @@ int dump_dir_accessible_by_uid(const char *dirname, uid_t uid)
return 0;
}
+
+void dd_mark_as_notreportable(struct dump_dir *dd, const char *reason)
+{
+ dd_save_text(dd, FILENAME_NOT_REPORTABLE, reason);
+}
--
1.8.1.2
11 years, 2 months
[LIBREPORT PATCH] logging: pass a single dot from event's process correctly
by Jakub Filak
- https://github.com/abrt/abrt/issues/607
Signed-off-by: Jakub Filak <jfilak(a)redhat.com>
---
src/cli/cli-report.c | 5 ++---
src/gui-wizard-gtk/wizard.c | 13 ++++++++++---
src/include/client.h | 3 +++
src/lib/client.c | 13 +++++++++++++
src/lib/run_event.c | 2 +-
5 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/src/cli/cli-report.c b/src/cli/cli-report.c
index ab4fb92..3923d60 100644
--- a/src/cli/cli-report.c
+++ b/src/cli/cli-report.c
@@ -472,7 +472,7 @@ struct logging_state {
static char *do_log(char *log_line, void *param)
{
- log("%s", log_line);
+ client_log(log_line);
return log_line;
}
@@ -481,8 +481,7 @@ static char *do_log_and_check_for_THANKYOU(char *log_line, void *param)
struct logging_state *l_state = param;
l_state->saw_THANKYOU |= (strcmp("THANKYOU", log_line) == 0);
l_state->output_was_produced |= (log_line[0] != '\0');
- log("%s", log_line);
- return log_line;
+ return do_log(log_line, param);
}
static int export_config_and_run_event(
diff --git a/src/gui-wizard-gtk/wizard.c b/src/gui-wizard-gtk/wizard.c
index b048823..4939dd2 100644
--- a/src/gui-wizard-gtk/wizard.c
+++ b/src/gui-wizard-gtk/wizard.c
@@ -1527,12 +1527,19 @@ static void terminate_event_chain()
static void update_command_run_log(const char* message, struct analyze_event_data *evd)
{
- gtk_label_set_text(g_lbl_event_log, message);
+ const bool it_is_a_dot = (message[0] == '.' && message[1] == '\0');
- char *log_msg = xasprintf("%s\n", message);
+ if (!it_is_a_dot)
+ gtk_label_set_text(g_lbl_event_log, message);
+
+ /* Don't append new line behind single dot */
+ const char *log_msg = it_is_a_dot ? message : xasprintf("%s\n", message);
append_to_textview(evd->tv_log, log_msg);
save_to_event_log(evd, log_msg);
- free(log_msg);
+
+ /* Because of single dot, see lines above */
+ if (log_msg != message)
+ free((void *)log_msg);
}
static void run_event_gtk_error(const char *error_line, void *param)
diff --git a/src/include/client.h b/src/include/client.h
index dc453b7..f2185d9 100644
--- a/src/include/client.h
+++ b/src/include/client.h
@@ -58,6 +58,9 @@ char *ask_password(const char *question);
#define alert libreport_alert
void alert(const char *message);
+#define client_log libreport_client_log
+void client_log(const char *message);
+
#ifdef __cplusplus
}
#endif
diff --git a/src/lib/client.c b/src/lib/client.c
index d1ada22..b7691d0 100644
--- a/src/lib/client.c
+++ b/src/lib/client.c
@@ -163,3 +163,16 @@ void alert(const char *message)
puts(message);
fflush(stdout);
}
+
+void client_log(const char *message)
+{
+ if (message != NULL
+ && (message[0] == '.' && message[1] == '\0')
+ && !is_slave_mode()
+ )
+ putchar('.');
+ else
+ printf("%s\n", message);
+
+ fflush(stdout);
+}
diff --git a/src/lib/run_event.c b/src/lib/run_event.c
index ea90029..6cd055c 100644
--- a/src/lib/run_event.c
+++ b/src/lib/run_event.c
@@ -744,7 +744,7 @@ char *run_event_stdio_ask_password(const char *msg, void *param)
static char *run_event_stdio_log(char *log_line, void *param)
{
- printf("%s\n", log_line);
+ client_log(log_line);
return log_line;
}
--
1.8.1.2
11 years, 2 months
[ABRT PATCH] dbus: add more debuging logs
by Jakub Filak
- closes #606
Signed-off-by: Jakub Filak <jfilak(a)redhat.com>
---
src/dbus/abrt-dbus.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c
index 754ef97..9ec4cdc 100644
--- a/src/dbus/abrt-dbus.c
+++ b/src/dbus/abrt-dbus.c
@@ -390,6 +390,7 @@ static struct dump_dir *open_directory_for_modification_of_element(
{
if (strcmp(*protected, element) == 0)
{
+ VERB1 log("'%s' element of '%s' can't be modified", element, problem_id);
char *error = xasprintf(_("'%s' element can't be modified"), element);
g_dbus_method_invocation_return_dbus_error(invocation,
"org.freedesktop.problems.ProtectedElement",
@@ -402,11 +403,17 @@ static struct dump_dir *open_directory_for_modification_of_element(
if (!dump_dir_accessible_by_uid(problem_id, caller_uid))
{
if (errno == ENOTDIR)
+ {
+ VERB1 log("'%s' is not a valid problem directory", problem_id);
return_InvalidProblemDir_error(invocation, problem_id);
+ }
else
+ {
+ VERB1 log("UID(%d) is not Authorized to access '%s'", caller_uid, problem_id);
g_dbus_method_invocation_return_dbus_error(invocation,
"org.freedesktop.problems.AuthFailure",
_("Not Authorized"));
+ }
return NULL;
}
@@ -414,6 +421,7 @@ static struct dump_dir *open_directory_for_modification_of_element(
struct dump_dir *dd = dd_opendir(problem_id, /* flags : */ 0);
if (!dd)
{ /* This should not happen because of the access check above */
+ VERB1 log("Can't access the problem '%s' for modification", problem_id);
g_dbus_method_invocation_return_dbus_error(invocation,
"org.freedesktop.problems.Failure",
_("Can't access the problem for modification"));
@@ -636,6 +644,7 @@ static void handle_method_call(GDBusConnection *connection,
if (element == NULL || element[0] == '\0' || strlen(element) > 64)
{
+ VERB1 log("'%s' is not a valid element name of '%s'", element, problem_id);
char *error = xasprintf(_("'%s' is not a valid element name"), element);
g_dbus_method_invocation_return_dbus_error(invocation,
"org.freedesktop.problems.InvalidElement",
@@ -648,16 +657,28 @@ static void handle_method_call(GDBusConnection *connection,
struct dump_dir *dd = open_directory_for_modification_of_element(
invocation, caller_uid, problem_id, element);
if (!dd)
+ /* Already logged from open_directory_for_modification_of_element() */
return;
/* Is it good idea to make it static? Is it possible to change the max size while a single run? */
- const double max_size = g_settings_nMaxCrashReportsSize * (1024 * 1024);
+ const double max_dir_size = g_settings_nMaxCrashReportsSize * (1024 * 1024);
+ const long item_size = dd_get_item_size(dd, element);
+ if (item_size < 0)
+ {
+ VERB1 log("Can't get size of '%s/%s'", problem_id, element);
+ char *error = xasprintf(_("Can't get size of '%s'"), element);
+ g_dbus_method_invocation_return_dbus_error(invocation,
+ "org.freedesktop.problems.Failure",
+ error);
+ return;
+ }
- const double requested_size = strlen(value) - dd_get_item_size(dd, element);
+ const double requested_size = strlen(value) - item_size;
/* Don't want to check the size limit in case of reducing of size */
if (requested_size > 0
- && requested_size > (max_size - get_dirsize(g_settings_dump_location)))
+ && requested_size > (max_dir_size - get_dirsize(g_settings_dump_location)))
{
+ VERB1 log("No problem space left in '%s' (requested Bytes %f)", problem_id, requested_size);
g_dbus_method_invocation_return_dbus_error(invocation,
"org.freedesktop.problems.Failure",
_("No problem space left"));
@@ -683,6 +704,7 @@ static void handle_method_call(GDBusConnection *connection,
struct dump_dir *dd = open_directory_for_modification_of_element(
invocation, caller_uid, problem_id, element);
if (!dd)
+ /* Already logged from open_directory_for_modification_of_element() */
return;
const int res = dd_delete_item(dd, element);
@@ -690,6 +712,7 @@ static void handle_method_call(GDBusConnection *connection,
if (res != 0)
{
+ VERB1 log("Can't delete the element '%s' from the problem directory '%s'", element, problem_id);
char *error = xasprintf(_("Can't delete the element '%s' from the problem directory '%s'"), element, problem_id);
g_dbus_method_invocation_return_dbus_error(invocation,
"org.freedesktop.problems.Failure",
--
1.8.1.2
11 years, 2 months
[PATCH] abrt-retrace-client: improve error messages
by Denys Vlasenko
Looked through abrt-retrace-client and related source files,
eyeing specifically error handling and messages.
The largest improvement comes from emitting URL
in calls to alert_connection_error(url), alert_server_error(url).
Also added URL printing to a few other messages.
Made some messages more precise:
s:"Cannot open temporary file":"Can't create temporary file in /tmp":
Added error check for tar and xz exit code.
Used xstat() where appropriate.
Removed a few PR_Close()s immediately before error_msg_and_die().
They were _sometimes_ present. No point in being inconsistent.
Open questions:
(1)
alert_connection_error(URL);
error_msg_and_die("Something horrible happened at '%s'", URL);
looks redundant. It looks like we _want_ error_msg_and_die
to also perform the duty of alert_connection_error.
(2) alert() needs to be made variadic a-la printf.
(3) I have a nagging desire to nuke all trailing periods in messages...
--
vda
diff -x '*.po' -d -urpN abrt.4/src/plugins/abrt-dedup-client.c abrt.5/src/plugins/abrt-dedup-client.c
--- abrt.4/src/plugins/abrt-dedup-client.c 2013-02-07 12:13:52.000000000 +0100
+++ abrt.5/src/plugins/abrt-dedup-client.c 2013-02-11 19:28:13.441262441 +0100
@@ -198,8 +198,7 @@ int main(int argc, char **argv)
0/*flags*/, PR_INTERVAL_NO_TIMEOUT);
if (written < 0)
{
- PR_Close(ssl_sock);
- alert_connection_error();
+ alert_connection_error(cfg.url);
error_msg_and_die(_("Failed to send HTTP request of length %d: NSS error %d."),
request->len, PR_GetError());
}
diff -x '*.po' -d -urpN abrt.4/src/plugins/abrt-retrace-client.c abrt.5/src/plugins/abrt-retrace-client.c
--- abrt.4/src/plugins/abrt-retrace-client.c 2013-02-07 16:44:13.135211195 +0100
+++ abrt.5/src/plugins/abrt-retrace-client.c 2013-02-11 18:02:31.923139578 +0100
@@ -97,7 +97,7 @@ static int create_archive(bool unlink_te
char *filename = xstrdup("/tmp/abrt-retrace-client-archive-XXXXXX.tar.xz");
int tempfd = mkstemps(filename, /*suffixlen:*/7);
if (tempfd == -1)
- perror_msg_and_die(_("Cannot open temporary file"));
+ perror_msg_and_die(_("Can't create temporary file in /tmp"));
if (unlink_temp)
xunlink(filename);
free(filename);
@@ -114,6 +114,7 @@ static int create_archive(bool unlink_te
int tar_xz_pipe[2];
xpipe(tar_xz_pipe);
+
fflush(NULL); /* paranoia */
pid_t xz_child = vfork();
if (xz_child == -1)
@@ -167,13 +168,25 @@ static int create_archive(bool unlink_te
free((void*)tar_args[2]);
close(tar_xz_pipe[1]);
- /* Wait for tar and xz to finish */
+ /* Wait for tar and xz to finish successfully */
+ int status;
VERB1 log_msg("Waiting for tar...");
- safe_waitpid(tar_child, NULL, 0);
+ safe_waitpid(tar_child, &status, 0);
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
+ {
+ /* Hopefully, by this time child emitted more meaningful
+ * error message. But just in case it didn't:
+ */
+ error_msg_and_die(_("Can't create temporary file in /tmp"));
+ }
VERB1 log_msg("Waiting for xz...");
- safe_waitpid(xz_child, NULL, 0);
-//FIXME: need to check that both exited with 0! Think about out-of-disk-space situation...
+ safe_waitpid(xz_child, &status, 0);
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
+ {
+ error_msg_and_die(_("Can't create temporary file in /tmp"));
+ }
VERB1 log_msg("Done...");
+
xlseek(tempfd, 0, SEEK_SET);
return tempfd;
}
@@ -195,9 +208,8 @@ struct retrace_settings *get_settings()
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
if (written == -1)
{
- PR_Close(ssl_sock);
- alert_connection_error();
- error_msg_and_die(_("Failed to send HTTP header of length %d: NSS error %d."),
+ alert_connection_error(cfg.url);
+ error_msg_and_die(_("Failed to send HTTP header of length %d: NSS error %d"),
http_request->len, PR_GetError());
}
strbuf_free(http_request);
@@ -208,7 +220,7 @@ struct retrace_settings *get_settings()
int response_code = http_get_response_code(http_response);
if (response_code != 200)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Unexpected HTTP response from server: %d\n%s"),
response_code, http_response);
}
@@ -217,7 +229,7 @@ struct retrace_settings *get_settings()
char *c, *row, *value;
if (!headers_end)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Invalid response from server: missing HTTP message body."));
}
row = headers_end + strlen("\r\n\r\n");
@@ -363,8 +375,7 @@ static int check_package(const char *nvr
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
if (written == -1)
{
- PR_Close(ssl_sock);
- alert_connection_error();
+ alert_connection_error(cfg.url);
error_msg_and_die(_("Failed to send HTTP header of length %d: NSS error %d"),
http_request->len, PR_GetError());
}
@@ -378,7 +389,7 @@ static int check_package(const char *nvr
if (response_code != 302 && response_code != 404)
{
char *http_body = http_get_body(http_response);
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Unexpected HTTP response from server: %d\n%s"),
response_code, http_body);
}
@@ -426,20 +437,16 @@ static int create(bool delete_temp_archi
/* get raw size */
if (coredump)
{
- if (stat(coredump, &file_stat) == -1)
- error_msg_and_die(_("Unable to stat file '%s'."), coredump);
-
+ xstat(coredump, &file_stat);
unpacked_size = (long long)file_stat.st_size;
}
else if (dump_dir_name != NULL)
{
struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags*/ 0);
if (!dd)
- error_msg_and_die(_("Unable to open dump directory '%s'."), dump_dir_name);
-
+ xfunc_die(); /* dd_opendir already emitted error message */
if (dd_exist(dd, FILENAME_VMCORE))
task_type = TASK_VMCORE;
-
dd_close(dd);
char *path;
@@ -448,13 +455,7 @@ static int create(bool delete_temp_archi
while (required_files[i])
{
path = concat_path_file(dump_dir_name, required_files[i]);
- if (stat(path, &file_stat) == -1)
- {
- error_msg(_("Unable to stat file '%s'."), path);
- free(path);
- xfunc_die();
- }
-
+ xstat(path, &file_stat);
free(path);
if (!S_ISREG(file_stat.st_mode))
@@ -507,7 +508,7 @@ static int create(bool delete_temp_archi
if (!supported)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("The server does not support "
"xz-compressed tarballs."));
}
@@ -652,9 +653,8 @@ static int create(bool delete_temp_archi
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
if (written == -1)
{
- PR_Close(ssl_sock);
- alert_connection_error();
- error_msg_and_die(_("Failed to send HTTP header of length %d: NSS error %d."),
+ alert_connection_error(cfg.url);
+ error_msg_and_die(_("Failed to send HTTP header of length %d: NSS error %d"),
http_request->len, PR_GetError());
}
@@ -711,7 +711,7 @@ static int create(bool delete_temp_archi
if the server send some explanation regarding the
error. */
result = 1;
- alert_connection_error();
+ alert_connection_error(cfg.url);
error_msg(_("Failed to send data: NSS error %d (%s): %s"),
PR_GetError(),
PR_ErrorToName(PR_GetError()),
@@ -732,7 +732,7 @@ static int create(bool delete_temp_archi
char *http_body = http_get_body(http_response);
if (!http_body)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Invalid response from server: missing HTTP message body."));
}
if (http_show_headers)
@@ -740,7 +740,7 @@ static int create(bool delete_temp_archi
int response_code = http_get_response_code(http_response);
if (response_code == 500 || response_code == 507)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(http_body);
}
else if (response_code == 403)
@@ -752,20 +752,20 @@ static int create(bool delete_temp_archi
}
else if (response_code != 201)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Unexpected HTTP response from server: %d\n%s"), response_code, http_body);
}
free(http_body);
*task_id = http_get_header_value(http_response, "X-Task-Id");
if (!*task_id)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Invalid response from server: missing X-Task-Id."));
}
*task_password = http_get_header_value(http_response, "X-Task-Password");
if (!*task_password)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Invalid response from server: missing X-Task-Password."));
}
free(http_response);
@@ -830,8 +830,7 @@ static void status(const char *task_id,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
if (written == -1)
{
- PR_Close(ssl_sock);
- alert_connection_error();
+ alert_connection_error(cfg.url);
error_msg_and_die(_("Failed to send HTTP header of length %d: NSS error %d"),
http_request->len, PR_GetError());
}
@@ -840,7 +839,7 @@ static void status(const char *task_id,
char *http_body = http_get_body(http_response);
if (!*http_body)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Invalid response from server: missing HTTP message body."));
}
if (http_show_headers)
@@ -848,14 +847,14 @@ static void status(const char *task_id,
int response_code = http_get_response_code(http_response);
if (response_code != 200)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Unexpected HTTP response from server: %d\n%s"),
response_code, http_body);
}
*task_status = http_get_header_value(http_response, "X-Task-Status");
if (!*task_status)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Invalid response from server: missing X-Task-Status."));
}
*status_message = http_body;
@@ -909,8 +908,7 @@ static void backtrace(const char *task_i
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
if (written == -1)
{
- PR_Close(ssl_sock);
- alert_connection_error();
+ alert_connection_error(cfg.url);
error_msg_and_die(_("Failed to send HTTP header of length %d: NSS error %d."),
http_request->len, PR_GetError());
}
@@ -919,7 +917,7 @@ static void backtrace(const char *task_i
char *http_body = http_get_body(http_response);
if (!http_body)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Invalid response from server: missing HTTP message body."));
}
if (http_show_headers)
@@ -927,7 +925,7 @@ static void backtrace(const char *task_i
int response_code = http_get_response_code(http_response);
if (response_code != 200)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Unexpected HTTP response from server: %d\n%s"),
response_code, http_body);
}
@@ -978,8 +976,7 @@ static void run_log(const char *task_id,
/*flags:*/0, PR_INTERVAL_NO_TIMEOUT);
if (written == -1)
{
- PR_Close(ssl_sock);
- alert_connection_error();
+ alert_connection_error(cfg.url);
error_msg_and_die(_("Failed to send HTTP header of length %d: NSS error %d."),
http_request->len, PR_GetError());
}
@@ -988,7 +985,7 @@ static void run_log(const char *task_id,
char *http_body = http_get_body(http_response);
if (!http_body)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Invalid response from server: missing HTTP message body."));
}
if (http_show_headers)
@@ -996,7 +993,7 @@ static void run_log(const char *task_id,
int response_code = http_get_response_code(http_response);
if (response_code != 200)
{
- alert_server_error();
+ alert_server_error(cfg.url);
error_msg_and_die(_("Unexpected HTTP response from server: %d\n%s"),
response_code, http_body);
}
diff -x '*.po' -d -urpN abrt.4/src/plugins/https-utils.c abrt.5/src/plugins/https-utils.c
--- abrt.4/src/plugins/https-utils.c 2013-02-07 12:13:52.000000000 +0100
+++ abrt.5/src/plugins/https-utils.c 2013-02-11 19:26:40.090426829 +0100
@@ -41,15 +41,28 @@ void get_language(struct language *lang)
++lang->encoding;
}
-void alert_server_error()
+void alert_server_error(const char *name)
{
- alert(_("An error occurred on the server side. Try again later."));
+ if (!name)
+ alert(_("An error occurred on the server side."));
+ else
+ {
+ char *msg = xasprintf(_("A server-side error occurred on '%s'"), name);
+ alert(msg);
+ free(msg);
+ }
}
-void alert_connection_error()
+void alert_connection_error(const char *name)
{
- alert(_("An error occurred while connecting to the server. "
- "Check your network connection and try again."));
+ if (!name)
+ alert(_("An error occurred while connecting to the server"));
+ else
+ {
+ char *msg = xasprintf(_("An error occurred while connecting to '%s'"), name);
+ alert(msg);
+ free(msg);
+ }
}
static SECStatus ssl_bad_cert_handler(void *arg, PRFileDesc *sock)
@@ -150,16 +163,18 @@ void ssl_connect(struct https_cfg *cfg,
PRAddrInfo *addrinfo = PR_GetAddrInfoByName(cfg->url, PR_AF_UNSPEC, PR_AI_ADDRCONFIG);
if (!addrinfo)
{
- alert_connection_error();
- error_msg_and_die(_("Failed to get host by name: NSS error %d."), PR_GetError());
+ alert_connection_error(cfg->url);
+ error_msg_and_die(_("Can't resolve host name '%s'. NSS error %d."), cfg->url, PR_GetError());
}
+ /* Hack */
+ ssl_allow_insecure = cfg->ssl_allow_insecure;
+
void *enumptr = NULL;
PRNetAddr addr;
*tcp_sock = NULL;
- ssl_allow_insecure = cfg->ssl_allow_insecure;
- while ((enumptr = PR_EnumerateAddrInfo(enumptr, addrinfo, cfg->port, &addr)))
+ while ((enumptr = PR_EnumerateAddrInfo(enumptr, addrinfo, cfg->port, &addr)) != NULL)
{
if (addr.raw.family == PR_AF_INET || addr.raw.family == PR_AF_INET6)
{
@@ -167,80 +182,64 @@ void ssl_connect(struct https_cfg *cfg,
break;
}
}
-
PR_FreeAddrInfo(addrinfo);
-
if (!*tcp_sock)
- error_msg_and_die(_("Failed to create a TCP socket"));
+ /* Host exists, but has neither IPv4 nor IPv6?? */
+ error_msg_and_die(_("Can't resolve host name '%s'."), cfg->url);
+
+ /* These operations are expected to always succeed: */
PRSocketOptionData sock_option;
- sock_option.option = PR_SockOpt_Nonblocking;
+ sock_option.option = PR_SockOpt_Nonblocking;
sock_option.value.non_blocking = PR_FALSE;
- PRStatus pr_status = PR_SetSocketOption(*tcp_sock, &sock_option);
- if (PR_SUCCESS != pr_status)
- {
- PR_Close(*tcp_sock);
+ if (PR_SUCCESS != PR_SetSocketOption(*tcp_sock, &sock_option))
error_msg_and_die(_("Failed to set socket blocking mode."));
- }
*ssl_sock = SSL_ImportFD(NULL, *tcp_sock);
if (!*ssl_sock)
- {
- PR_Close(*tcp_sock);
error_msg_and_die(_("Failed to wrap TCP socket by SSL."));
- }
- SECStatus sec_status = SSL_OptionSet(*ssl_sock, SSL_HANDSHAKE_AS_CLIENT, PR_TRUE);
- if (SECSuccess != sec_status)
- {
- PR_Close(*ssl_sock);
+ if (SECSuccess != SSL_OptionSet(*ssl_sock, SSL_HANDSHAKE_AS_CLIENT, PR_TRUE))
error_msg_and_die(_("Failed to enable client handshake to SSL socket."));
- }
-
if (SECSuccess != SSL_OptionSet(*ssl_sock, SSL_ENABLE_SSL2, PR_TRUE))
error_msg_and_die(_("Failed to enable client handshake to SSL socket."));
if (SECSuccess != SSL_OptionSet(*ssl_sock, SSL_ENABLE_SSL3, PR_TRUE))
error_msg_and_die(_("Failed to enable client handshake to SSL socket."));
if (SECSuccess != SSL_OptionSet(*ssl_sock, SSL_ENABLE_TLS, PR_TRUE))
error_msg_and_die(_("Failed to enable client handshake to SSL socket."));
-
- sec_status = SSL_SetURL(*ssl_sock, cfg->url);
- if (SECSuccess != sec_status)
- {
- PR_Close(*ssl_sock);
+ if (SECSuccess != SSL_SetURL(*ssl_sock, cfg->url))
error_msg_and_die(_("Failed to set URL to SSL socket."));
- }
- pr_status = PR_Connect(*ssl_sock, &addr, PR_INTERVAL_NO_TIMEOUT);
- if (PR_SUCCESS != pr_status)
+
+ /* This finally sends packets down the wire.
+ * If we fail here, then server denied our connect, or is down, etc.
+ * Need a good error message.
+ */
+ if (PR_SUCCESS != PR_Connect(*ssl_sock, &addr, PR_INTERVAL_NO_TIMEOUT))
{
- PR_Close(*ssl_sock);
- alert_connection_error();
- error_msg_and_die(_("Failed to connect SSL address."));
+ alert_connection_error(cfg->url);
+ error_msg_and_die(_("Can't connect to '%s'"), cfg->url);
}
+
+ /* These should not fail either. (Why we don't set them earlier?) */
if (SECSuccess != SSL_BadCertHook(*ssl_sock,
(SSLBadCertHandler)ssl_bad_cert_handler,
NULL))
{
- PR_Close(*ssl_sock);
error_msg_and_die(_("Failed to set certificate hook."));
}
if (SECSuccess != SSL_HandshakeCallback(*ssl_sock,
(SSLHandshakeCallback)ssl_handshake_callback,
NULL))
{
- PR_Close(*ssl_sock);
error_msg_and_die(_("Failed to set handshake callback."));
}
- sec_status = SSL_ResetHandshake(*ssl_sock, /*asServer:*/PR_FALSE);
- if (SECSuccess != sec_status)
+ if (SECSuccess != SSL_ResetHandshake(*ssl_sock, /*asServer:*/PR_FALSE))
{
- PR_Close(*ssl_sock);
- alert_server_error();
error_msg_and_die(_("Failed to reset handshake."));
}
- sec_status = SSL_ForceHandshake(*ssl_sock);
- if (SECSuccess != sec_status)
+
+ /* This performs SSL/TLS negotiation */
+ if (SECSuccess != SSL_ForceHandshake(*ssl_sock))
{
- PR_Close(*ssl_sock);
- alert_server_error();
- error_msg_and_die(_("Failed to force handshake: NSS error %d."),
+ alert_connection_error(cfg->url);
+ error_msg_and_die(_("Failed to complete SSL handshake: NSS error %d."),
PR_GetError());
}
}
@@ -301,23 +300,20 @@ char *http_get_body(const char *message)
int http_get_response_code(const char *message)
{
if (0 != strncmp(message, "HTTP/", strlen("HTTP/")))
- {
- alert_server_error();
- error_msg_and_die(_("Invalid response from server: HTTP header not found."));
- }
- char *space = strstr(message, " ");
+ goto err;
+ char *space = strchr(message, ' ');
if (!space)
- {
- alert_server_error();
- error_msg_and_die(_("Invalid response from server: HTTP header not found."));
- }
+ goto err;
int response_code;
if (1 != sscanf(space + 1, "%d", &response_code))
- {
- alert_server_error();
- error_msg_and_die(_("Invalid response from server: HTTP header not found."));
- }
+ goto err;
return response_code;
+
+ err:
+ alert_server_error(NULL);
+ /* Show bad header to the user */
+ char *sanitized = sanitize_utf8(message, (SANITIZE_ALL & ~SANITIZE_TAB));
+ error_msg_and_die(_("Malformed HTTP response header: '%s'"), sanitized ? sanitized : message);
}
void http_print_headers(FILE *file, const char *message)
@@ -353,7 +349,7 @@ char *tcp_read_response(PRFileDesc *tcp_
}
if (received == -1)
{
- alert_connection_error();
+ alert_connection_error(NULL);
error_msg_and_die(_("Receiving of data failed: NSS error %d."),
PR_GetError());
}
diff -x '*.po' -d -urpN abrt.4/src/plugins/https-utils.h abrt.5/src/plugins/https-utils.h
--- abrt.4/src/plugins/https-utils.h 2013-02-07 12:13:52.000000000 +0100
+++ abrt.5/src/plugins/https-utils.h 2013-02-11 18:00:59.896317431 +0100
@@ -49,8 +49,8 @@ struct https_cfg
};
void get_language(struct language *lang);
-void alert_server_error();
-void alert_connection_error();
+void alert_server_error(const char *peer_name);
+void alert_connection_error(const char *peer_name);
void ssl_connect(struct https_cfg *cfg, PRFileDesc **tcp_sock, PRFileDesc **ssl_sock);
void ssl_disconnect(PRFileDesc *ssl_sock);
char *http_get_header_value(const char *message, const char *header_name);
11 years, 2 months
[LIBREPORT PATCH] dd: convert time at lock time
by Jakub Filak
- closes #123
Signed-off-by: Jakub Filak <jfilak(a)redhat.com>
---
src/include/dump_dir.h | 1 +
src/lib/dump_dir.c | 68 ++++++++++++++++++++++++++++++++++----------------
2 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/src/include/dump_dir.h b/src/include/dump_dir.h
index 21169c8..63c0639 100644
--- a/src/include/dump_dir.h
+++ b/src/include/dump_dir.h
@@ -45,6 +45,7 @@ struct dump_dir {
gid_t dd_gid;
/* mode fo saved files */
mode_t mode;
+ time_t dd_time;
};
void dd_close(struct dump_dir *dd);
diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
index f7732ea..2736ba9 100644
--- a/src/lib/dump_dir.c
+++ b/src/lib/dump_dir.c
@@ -109,18 +109,19 @@ static bool exist_file_dir(const char *path)
return false;
}
-/* Returns true if the file is not readable or */
-/* if the file doesn't contain valid unixt time stamp */
-/* implementation is a compromise between efficiency and accuracy */
-/* the function gets false negative results for big numbers */
-static bool not_valid_time_file(const char *filename)
+/* Returns value less than 0 if the file is not readable or
+ * if the file doesn't contain valid unixt time stamp.
+ *
+ * Any possible failure will be logged.
+ */
+static long int parse_time_file(const char *filename)
{
/* Open input file, and parse it. */
int fd = open(filename, O_RDONLY | O_NOFOLLOW);
if (fd < 0)
{
VERB2 perror_msg("Can't open '%s'", filename);
- return true;
+ return -1;
}
/* ~ maximal number of digits for positive time stamp string*/
@@ -133,18 +134,17 @@ static bool not_valid_time_file(const char *filename)
if (rdsz == -1)
{
VERB2 perror_msg("Can't read from '%s'", filename);
- return true;
+ return -1;
}
-
/* approximate maximal number of digits in timestamp is sizeof(time_t)*3 */
/* buffer has this size + 1 byte for trailing '\0' */
/* if whole size of buffer was read then file is bigger */
/* than string representing maximal time stamp */
if (rdsz == sizeof(time_buf))
{
- VERB2 log("File '%s' is too long to be valid unix "
- "time stamp (max size %zdB)", filename, sizeof(time_buf));
- return true;
+ VERB2 error_msg("File '%s' is too long to be valid unix "
+ "time stamp (max size %zdB)", filename, sizeof(time_buf));
+ return -1;
}
/* Our tools don't put trailing newline into time file,
@@ -154,16 +154,38 @@ static bool not_valid_time_file(const char *filename)
rdsz--;
time_buf[rdsz] = '\0';
- /* the value should fit to timestamp number range because of file size */
- /* condition above, hence check if the value string consists only from digits */
- if (!isdigit_str(time_buf))
+ errno = 0; /* To distinguish success/failure after call */
+ char *endptr;
+ long val = strtol(time_buf, &endptr, /* base */ 10);
+
+ /* Check for various possible errors */
+ if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
+ || (errno != 0 && val == 0))
{
- VERB2 log("File '%s' doesn't contain valid unix "
- "time stamp ('%s')", filename, time_buf);
- return true;
+ VERB2 perror_msg("File '%s' doesn't contain valid unix "
+ "time stamp ('%s')", filename, time_buf);
+ return -1;
}
- return false;
+ if (endptr == time_buf)
+ {
+ VERB2 error_msg("No digits were found in file '%s'", filename);
+ return -1;
+ }
+
+ if (*endptr != '\0')
+ {
+ VERB2 error_msg("Further characters after number: '%s' in file '%s'", endptr, filename);
+ return -1;
+ }
+
+ if (val < 0)
+ {
+ VERB2 error_msg("Negative time found in file '%s'", filename);
+ }
+
+ /* If we got here, strtol() successfully parsed a number */
+ return val;
}
/* Return values:
@@ -260,7 +282,8 @@ static int dd_lock(struct dump_dir *dd, unsigned sleep_usec, int flags)
if (sleep_usec == WAIT_FOR_OTHER_PROCESS_USLEEP) /* yes */
{
strcpy(lock_buf + dirname_len, "/"FILENAME_TIME);
- if (not_valid_time_file(lock_buf))
+ dd->dd_time = parse_time_file(lock_buf);
+ if (dd->dd_time < 0)
{
/* time file doesn't exist. We managed to lock the directory
* which was just created by somebody else, or is almost deleted
@@ -302,7 +325,9 @@ static void dd_unlock(struct dump_dir *dd)
static inline struct dump_dir *dd_init(void)
{
- return (struct dump_dir*)xzalloc(sizeof(struct dump_dir));
+ struct dump_dir* dd = (struct dump_dir*)xzalloc(sizeof(struct dump_dir));
+ dd->dd_time = -1;
+ return dd;
}
int dd_exist(const struct dump_dir *dd, const char *path)
@@ -361,7 +386,8 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
&& access(dir, R_OK) == 0
) {
char *time_file_name = concat_path_file(dir, FILENAME_TIME);
- if (not_valid_time_file(time_file_name))
+ dd->dd_time = parse_time_file(time_file_name);
+ if (dd->dd_time < 0)
{
dd_close(dd);
dd = NULL;
--
1.8.1.2
11 years, 2 months