From 0689f7a0464a27247cbb78b1a9ecf3adef7cda90 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Tue, 23 Mar 2021 12:29:18 +0100 Subject: [PATCH] bugreport-service: Add return error code about no process for livedump_pid() method Change-Id: I9f0182b9b857c4aca685d495f568c1fbd48a8b37 --- packaging/libbugreport.spec | 2 +- src/bugreport-service/bugreport-service.c | 83 +++++++++++++++++++------------ src/bugreport-service/systemdump.c | 5 +- src/bugreport-service/systemdump.h | 2 +- src/bugreport/libbugreport.h | 9 ++++ 5 files changed, 64 insertions(+), 37 deletions(-) diff --git a/packaging/libbugreport.spec b/packaging/libbugreport.spec index f1d7d83..4ce2d90 100644 --- a/packaging/libbugreport.spec +++ b/packaging/libbugreport.spec @@ -3,7 +3,7 @@ Name: libbugreport Summary: libbugreport provides API to communicate with bugreport-service -Version: 1.0.0 +Version: 1.0.1 Release: 1 Provides: libcrash-service = %{version}-%{release} Group: Framework/system diff --git a/src/bugreport-service/bugreport-service.c b/src/bugreport-service/bugreport-service.c index 002149b..54ba621 100644 --- a/src/bugreport-service/bugreport-service.c +++ b/src/bugreport-service/bugreport-service.c @@ -28,6 +28,7 @@ #include #include #include +#include #include "bugreport-service.h" @@ -51,13 +52,6 @@ #define TIMEOUT_INTERVAL_SEC 60 #define TIMEOUT_LIVEDUMP_SEC 50 -#define CS_ERROR 1 -#define CS_ERR_PARAM 1 -#define CS_ERR_TIMEOUT 2 -#define CS_ERR_READ 3 -#define CS_ERR_INTERNAL 4 -#define CS_ERR_UNSUPPORTED 5 - static GMainLoop *loop; static GMutex timeout_mutex; static guint timeout_id; @@ -145,6 +139,7 @@ static gboolean read_result_cb(gpointer data) { struct callback_data *cb_data = (struct callback_data*)data; char report_path[PATH_MAX] = {0}; + bool error_sent = false; if (!data_ready(cb_data->read_fd)) { _I("Report is not ready after %d seconds.", TIMEOUT_LIVEDUMP_SEC); @@ -153,53 +148,78 @@ static gboolean read_result_cb(gpointer data) CS_ERR_TIMEOUT, "Report is not ready"); kill(cb_data->child_pid, SIGKILL); - goto end; - } else { - if (read(cb_data->read_fd, report_path, sizeof(report_path)) == -1) { - _E("Read from child error: %m"); + error_sent = true; + } else if (read(cb_data->read_fd, report_path, sizeof(report_path)) == -1) { + _E("Read from child error: %m"); + g_dbus_method_invocation_return_error(cb_data->invocation, + CS_ERROR, + CS_ERR_READ, + "Error while obtaining report path"); + error_sent = true; + } + + close(cb_data->read_fd); + int status; + if (waitpid(cb_data->child_pid, &status, 0) == -1 && !error_sent) { + g_dbus_method_invocation_return_error(cb_data->invocation, + CS_ERROR, + CS_ERR_INTERNAL, + "Internal error"); + error_sent = true; + } + + if (!error_sent && WIFEXITED(status)) { + int exit_status = WEXITSTATUS(status); + if (exit_status != CS_ERR_NO_ERROR) { + _E("Child process returned an error code: %d", exit_status); g_dbus_method_invocation_return_error(cb_data->invocation, - CS_ERROR, - CS_ERR_READ, - "Error while obtaining report path"); - goto end; + CS_ERROR, + exit_status, + "Error"); + error_sent = true; } } - int res = diagnostics_send_event("BugreportCreated", NULL); - if (res != DIAGNOSTICS_ERROR_NONE) - _E("Send diagnostics event error: %d", res); + if (!error_sent) { + int res = diagnostics_send_event("BugreportCreated", NULL); + if (res != DIAGNOSTICS_ERROR_NONE) + _E("Send diagnostics event error: %d", res); - g_dbus_method_invocation_return_value(cb_data->invocation, - g_variant_new("(s)", report_path)); -end: - close(cb_data->read_fd); - - waitpid(cb_data->child_pid, NULL, 0); + g_dbus_method_invocation_return_value(cb_data->invocation, + g_variant_new("(s)", report_path)); + } free(cb_data); return G_SOURCE_REMOVE; } -static bool livedump_run(struct dump_operation_data *data) +static int livedump_run(struct dump_operation_data *data) { assert(data); + + if (kill(data->pid, 0) != 0) { + close(data->write_fd); + _E("No such process: %d", data->pid); + return CS_ERR_NO_PROC; + } + char report_path[PATH_MAX]; if (!crash_manager_livedump_pid(data->pid, data->dump_reason, report_path, sizeof(report_path))) { _E("crash_manager_livedump_pid error"); - return false; + return CS_ERR_INTERNAL; } if (write(data->write_fd, report_path, strlen(report_path) + 1) == -1) { _E("Write report_path error: %m"); close(data->write_fd); - return false; + return CS_ERR_INTERNAL; } close(data->write_fd); - return true; + return CS_ERR_NO_ERROR; } -static void dump_handler(GDBusMethodInvocation *invocation, bool (*operation)(struct dump_operation_data *), struct dump_operation_data *data) +static void dump_handler(GDBusMethodInvocation *invocation, int (*operation)(struct dump_operation_data *), struct dump_operation_data *data) { /* * We want to run the livedump in different process so as not to @@ -235,10 +255,7 @@ static void dump_handler(GDBusMethodInvocation *invocation, bool (*operation)(st if (child_pid == 0) { close(fds[0]); data->write_fd = fds[1]; - if (operation(data)) - exit(EXIT_SUCCESS); - else - exit(EXIT_FAILURE); + exit(operation(data)); } else if (child_pid > 0) { cb_data->child_pid = child_pid; close(fds[1]); diff --git a/src/bugreport-service/systemdump.c b/src/bugreport-service/systemdump.c index b9a7a10..bfc899c 100644 --- a/src/bugreport-service/systemdump.c +++ b/src/bugreport-service/systemdump.c @@ -22,6 +22,7 @@ #include #include +#include "libbugreport.h" #include "bugreport-service.h" #include "systemdump.h" @@ -153,7 +154,7 @@ static bool move_report(struct systemstate_info *sinfo) return is_ok; } -bool systemstate_dump_run(struct dump_operation_data *data) +int systemstate_dump_run(struct dump_operation_data *data) { bool is_ok = true; @@ -183,5 +184,5 @@ exit: close(data->write_fd); remove_dir(sinfo.temp_path, true); systemstate_info_free(&sinfo); - return is_ok; + return is_ok ? CS_ERR_NO_ERROR : CS_ERR_INTERNAL; } diff --git a/src/bugreport-service/systemdump.h b/src/bugreport-service/systemdump.h index 72172f3..68ca925 100644 --- a/src/bugreport-service/systemdump.h +++ b/src/bugreport-service/systemdump.h @@ -27,6 +27,6 @@ struct dump_operation_data { const char *dump_reason; }; -bool systemstate_dump_run(struct dump_operation_data *dump); +int systemstate_dump_run(struct dump_operation_data *dump); #endif // __SYSTEMDUMP_H diff --git a/src/bugreport/libbugreport.h b/src/bugreport/libbugreport.h index cfa6d5b..92782e5 100644 --- a/src/bugreport/libbugreport.h +++ b/src/bugreport/libbugreport.h @@ -18,6 +18,15 @@ #include #include +#define CS_ERROR 1 +#define CS_ERR_NO_ERROR 0 +#define CS_ERR_PARAM 1 +#define CS_ERR_TIMEOUT 2 +#define CS_ERR_READ 3 +#define CS_ERR_INTERNAL 4 +#define CS_ERR_UNSUPPORTED 5 +#define CS_ERR_NO_PROC 6 + /** * @brief Sends a request to the bugreport-service to create livedump report of specific process. * @param pid PID of process for which the report should be created. -- 2.7.4