From 918ec37ac5024dcec1dd9b34ee86254e476c0eb4 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Tue, 25 Sep 2018 13:38:58 +0200 Subject: [PATCH 01/16] util: Allow functions to be called from c++ Change-Id: If4dc6b6e494096713e76a1345ab3753a6914e1c2 --- src/shared/util.h | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/shared/util.h b/src/shared/util.h index c4aa1a6..03de693 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -23,6 +23,12 @@ #define __CONSTRUCTOR__ __attribute__ ((constructor)) #endif +#define fprintf_fd(fd, fmt, ...) dprintf(fd, fmt, ##__VA_ARGS__) + +#ifdef __cplusplus +extern "C" { +#endif + int system_command(char *command); int system_command_with_timeout(int timeout_seconds, char *command); @@ -31,8 +37,6 @@ int system_command_parallel(char *command); int wait_system_command(int pid); -#define fprintf_fd(fd, fmt, ...) dprintf(fd, fmt, ##__VA_ARGS__) - int write_fd(int fd, const void *buf, int len); int copy_file(char *src, char *dst); @@ -62,6 +66,11 @@ char* get_exe_path(pid_t pid); char* get_cmd_line(pid_t pid); char *nullvec2str(char *const vec[]); + +#ifdef __cplusplus +} +#endif + /** * @} */ -- 2.7.4 From 6507ecd5e1e40d136c1272ce8b9b80fac82f4f67 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 3 Oct 2018 11:27:57 +0200 Subject: [PATCH 02/16] util: Remove unused file_exist() Change-Id: I1e20fc6cd55c9685db304a9aa40b58af56df0c00 --- src/shared/util.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/shared/util.c b/src/shared/util.c index b47fd02..8273ca2 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -146,17 +146,6 @@ int system_command_with_timeout(int timeout_seconds, char *command) } } -int file_exist(const char *file) -{ - FILE *fp; - - fp = fopen(file, "r"); - if (fp == NULL) - return -1; - fclose(fp); - return 1; -} - int write_fd(int fd, const void *buf, int len) { int count; -- 2.7.4 From 7f46a77f84b58234616885af19255166b273a390 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 3 Oct 2018 11:29:30 +0200 Subject: [PATCH 03/16] Fix: Do not index buffer[] with negative index in case of read failure Issue reported by Coverity. Change-Id: Ie9cd66d2801770a096bb1849d11c860533508174 --- src/shared/util.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/shared/util.c b/src/shared/util.c index 8273ca2..e6a2cff 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -742,14 +742,13 @@ char* get_cmd_line(pid_t pid) char buffer[PATH_MAX]; ssize_t ret = read(fd, buffer, sizeof(buffer) - 1); - buffer[ret] = '\0'; close(fd); - if (ret <= 0) { _E("Failed to read %s: %m\n", cmdline_path); return NULL; } + buffer[ret] = '\0'; char *result; if (asprintf(&result, "%s", buffer) == -1) { -- 2.7.4 From e293ed8ad6ee9ee57f5d909de19af5a7f0fff8e3 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 3 Oct 2018 11:35:46 +0200 Subject: [PATCH 04/16] Release 5.0.6 This change brings one fix and set of assorted cleanups: - Fix: Do not index buffer[] with negative index in case of read failure - util: Remove unused file_exist() - util: Allow functions to be called from c++ - util: Add nullvec2str - util: Remove unused cat_file() - util: Dead code removal - crash-manager: Use same constant for minicoredumper, crash-stack and zip command timeouts Change-Id: I46fbdea06ad36f06116751c5f14105350569cad5 --- packaging/crash-worker.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index c32e7db..6104aea 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -10,7 +10,7 @@ Name: crash-worker Summary: Crash-manager -Version: 5.0.5 +Version: 5.0.6 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From bddf2ca3744ee97abb258ddfbc1b95fe18a43047 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 3 Oct 2018 15:43:16 +0200 Subject: [PATCH 05/16] Fix: Another instance of indexing array with negative index Issue reported by Coverity. Change-Id: Ida7f3106d650f48d5fff7e976f1bbc0df0fdc308 --- src/shared/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/util.c b/src/shared/util.c index e6a2cff..b83dbaa 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -768,12 +768,12 @@ char* get_exe_path(pid_t pid) "/proc/%d/exe", pid); ssize_t ret = readlink(exe_link, buffer, sizeof(buffer) - 1); - buffer[ret] = '\0'; if (ret <= 0) { _E("Failed to read link %s: %m", exe_link); return NULL; } + buffer[ret] = '\0'; if (access(buffer, F_OK) == -1) { _E("Invalid path %s", buffer); -- 2.7.4 From ad7c58644a30ccba6e2cd95d071688f073f0e152 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 3 Oct 2018 15:44:20 +0200 Subject: [PATCH 06/16] Release 5.0.7 This release brings one addtional fix for indexing array with negative index bug. Change-Id: I9e49379a68f4839256d1df26e42b144b6ed6a708 --- packaging/crash-worker.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index 6104aea..7f1cf41 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -10,7 +10,7 @@ Name: crash-worker Summary: Crash-manager -Version: 5.0.6 +Version: 5.0.7 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From 9b9190f5e41257a244b5e254c38e5c595d33f4a8 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 3 Oct 2018 13:14:20 +0200 Subject: [PATCH 07/16] crash-manager: Do not make temporary copy of crashed process maps It was needed when crash-worker used (now removed) ptrace -based crash-stack, which woken up crashed process, causing /proc// entries to disappear. Change-Id: Iec344e6c3b9d7d12979173dea530684573a0761d --- src/crash-manager/crash-manager.c | 35 ++++++----------------------------- 1 file changed, 6 insertions(+), 29 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index af57938..686a13e 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -77,7 +77,6 @@ #define MINICOREDUMPER_TIMEOUT DEFAULT_COMMAND_TIMEOUT #define CRASH_STACK_TIMEOUT DEFAULT_COMMAND_TIMEOUT #define ZIP_TIMEOUT DEFAULT_COMMAND_TIMEOUT -#define TEMP_MAPS_FILENAME "maps.temp" enum { RET_EXCEED = 1, @@ -703,35 +702,12 @@ static int dump_system_state(const struct crash_info *cinfo) return system_command_parallel(command); } -static void copy_maps(const struct crash_info *cinfo) -{ - char maps_path[PATH_MAX]; - char temp_maps_path[PATH_MAX]; - snprintf(maps_path, sizeof(maps_path), "/proc/%d/maps", - cinfo->pid_info); - - snprintf(temp_maps_path, sizeof(temp_maps_path), "%s/%s", - cinfo->temp_dir, TEMP_MAPS_FILENAME); - - copy_file(maps_path, temp_maps_path); -} - -static void remove_maps(const struct crash_info *cinfo) -{ - char temp_maps_path[PATH_MAX]; - - snprintf(temp_maps_path, sizeof(temp_maps_path), "%s/%s", - cinfo->temp_dir, TEMP_MAPS_FILENAME); - if (unlink(temp_maps_path) < 0) - _E("Cannot remove %s: %m\n", temp_maps_path); -} - static void save_so_info(const struct crash_info *cinfo) { char maps_path[PATH_MAX]; char so_info_path[PATH_MAX]; - snprintf(maps_path, sizeof(maps_path), "%s/%s", - cinfo->temp_dir, TEMP_MAPS_FILENAME); + snprintf(maps_path, sizeof(maps_path), "/proc/%d/maps", + cinfo->pid_info); snprintf(so_info_path, sizeof(so_info_path), "%s/%s.%s", cinfo->pfx, cinfo->name, "so_info"); @@ -1278,8 +1254,10 @@ int main(int argc, char *argv[]) /* Exec dump_systemstate */ dump_state_pid = dump_system_state(&cinfo); - /* Copy maps file to temp dir */ - copy_maps(&cinfo); + if (dump_state_pid < 0) { + res = EXIT_FAILURE; + goto exit; + } } /* Exec crash modules */ if (execute_crash_modules(&cinfo) < 0) { @@ -1290,7 +1268,6 @@ int main(int argc, char *argv[]) if (report_type >= REP_TYPE_FULL) { /* Save shared objects info (file names, bulid IDs, rpm package names) */ save_so_info(&cinfo); - remove_maps(&cinfo); /* Wait dump_system_state */ wait_system_command(dump_state_pid); -- 2.7.4 From d23978584db9f97fa516c1c64a7bd355962a0467 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Mon, 15 Oct 2018 13:43:15 +0200 Subject: [PATCH 08/16] Handle spaces in the crashed app pathname and in the maps file Change-Id: I98ff537cab9c37b78dc3f3056a375fd10461b7ea --- src/crash-manager/crash-manager.c | 2 +- src/crash-stack/crash-stack.c | 7 ++++++- src/crash-stack/unwind.c | 8 +++++++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 686a13e..a02d59c 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -692,7 +692,7 @@ static int dump_system_state(const struct crash_info *cinfo) char command[PATH_MAX]; ret = snprintf(command, sizeof(command), - "/usr/bin/dump_systemstate -d -k -j -f %s", + "/usr/bin/dump_systemstate -d -k -j -f '%s'", cinfo->log_path); if (ret < 0) { _E("Failed to snprintf for dump_systemstate command"); diff --git a/src/crash-stack/crash-stack.c b/src/crash-stack/crash-stack.c index bf1cd38..61b876c 100644 --- a/src/crash-stack/crash-stack.c +++ b/src/crash-stack/crash-stack.c @@ -62,6 +62,11 @@ #define STRING_FORMAT_SPECIFIER_WITH_MACRO(macro) "%"#macro"s" #define STR_FS(macro) STRING_FORMAT_SPECIFIER_WITH_MACRO(macro) +// this macro returns the format specifier to capture a string that can +// contain spaces +#define STRINGWW_FORMAT_SPECIFIER_WITH_MACRO(macro) "%"#macro"[^\t\n]" +#define STRWW_FS(macro) STRINGWW_FORMAT_SPECIFIER_WITH_MACRO(macro) + static FILE *outputfile = NULL; ///< global output stream static FILE *errfile = NULL; ///< global error stream static FILE *bufferfile = NULL; ///< buffer file for ordering @@ -377,7 +382,7 @@ static struct addr_node *get_addr_list_from_maps(int fd) result = sscanf(linebuf, STR_FS(ADDR_LEN) STR_FS(PERM_LEN) "%*s %*s %*s" - STR_FS(PATH_MAX), addr, perm, path); + STRWW_FS(PATH_MAX), addr, perm, path); if (result < 0) continue; perm[PERM_LEN] = 0; diff --git a/src/crash-stack/unwind.c b/src/crash-stack/unwind.c index bce9ef8..d372bbe 100644 --- a/src/crash-stack/unwind.c +++ b/src/crash-stack/unwind.c @@ -861,7 +861,13 @@ void *_TB_create (pid_t pid) { if (fgets(buf, STAT_LINE_MAXSIZE, f) == NULL) goto out_err; - for (i = 0, p = buf; i < 28; i++) { + // The filename of the executable (the second field) is in parentheses and can + // contain whitespaces. Therefore I'm looking for the last ')' character + // and then iterate to the 27 field from here. + p = strrchr(buf, ')'); + if (!p) + goto out_err; + for (i = 0; i < 27; i++) { p = strchr(p, ' '); if (!p) goto out_err; -- 2.7.4 From a6ba03c4c82269ff20c12a277b99b7d765ecdea3 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Wed, 17 Oct 2018 13:23:29 +0200 Subject: [PATCH 09/16] dump_systemstate: Continue dumping after failure of a subcommand Change-Id: I7c0481984fbb9c04b26f913fb25208b11f9d6869 --- src/dump_systemstate/dump_systemstate.c | 53 ++++++++++++++------------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/src/dump_systemstate/dump_systemstate.c b/src/dump_systemstate/dump_systemstate.c index 776ec2b..fb2ee75 100644 --- a/src/dump_systemstate/dump_systemstate.c +++ b/src/dump_systemstate/dump_systemstate.c @@ -84,7 +84,13 @@ static int get_disk_used_percent(const char *path) int main(int argc, char *argv[]) { - int c, ret, i, is_root, dpercent; +#define INFORM_IF_ERROR(ret) \ + if (ret < 0) { \ + exit_code = ret; \ + fprintf_fd(out_fd, "\nCommand failed with error code: %d", ret); \ + } + + int c, ret, i, is_root, dpercent, exit_code = 0; const char *arg_file = NULL; int out_fd = -1; bool arg_dlog = false; @@ -130,7 +136,7 @@ int main(int argc, char *argv[]) out_fd = open(arg_file, O_WRONLY | O_TRUNC | O_CREAT, FILE_PERM); if (out_fd < 0) { perror("couldn't open output file"); - ret = out_fd; + exit_code = out_fd; goto exit; } } @@ -144,66 +150,56 @@ int main(int argc, char *argv[]) fprintf_fd(out_fd, "\n%s(%s)\n", dump_item[i].title, dump_item[i].path); ret = dump_file_write_fd((char *)dump_item[i].path, out_fd); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) } fprintf_fd(out_fd, "\n"); fprintf_fd(out_fd, "\n==== System disk space usage (/bin/df -h)\n"); char *df_args[] = {"/bin/df", "-h", NULL}; ret = run_command_write_fd_timeout(df_args[0], df_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) dpercent = get_disk_used_percent("/opt"); if (90 < dpercent) { fprintf_fd(out_fd, "\n==== System disk space usage detail - %d%% (/bin/du -ah /opt)\n", dpercent); char *du_args[] = {"/bin/du", "-ah", "/opt", "--exclude=/opt/usr", NULL}; ret = run_command_write_fd_timeout(du_args[0], du_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) } fprintf_fd(out_fd, "\n==== System timezone (ls -al /opt/etc/localtime)\n"); char *ls_args[] = {"/bin/ls", "-al", "/opt/etc/localtime", NULL}; ret = run_command_write_fd_timeout(ls_args[0], ls_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) fprintf_fd(out_fd, "\n==== System summary (/usr/bin/top -bcH -n 1)\n"); char *top_args[] = {"/bin/top", "-bcH", "-n", "1", NULL}; char *top_env[] = {"COLUMNS=200", NULL}; ret = run_command_write_fd_timeout(top_args[0], top_args, top_env, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) fprintf_fd(out_fd, "\n==== Current processes (/bin/ps auxfw)\n"); char *ps_args[] = {"/bin/ps", "auxfw", NULL}; ret = run_command_write_fd_timeout(ps_args[0], ps_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) fprintf_fd(out_fd, "\n==== System memory statistics (/usr/bin/memps -v)\n"); char *memps_args[] = {"/bin/memps", "-v", NULL}; ret = run_command_write_fd_timeout(memps_args[0], memps_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) if (is_root) { fprintf_fd(out_fd, "\n==== System configuration (/usr/bin/vconftool get memory, db, file)\n"); char *get_mem_args[] = {"/bin/vconftool", "get", "memory/", "-r", NULL}; ret = run_command_write_fd_timeout(get_mem_args[0], get_mem_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) char *get_db_args[] = {"/bin/vconftool", "get", "db/", "-r", NULL}; ret = run_command_write_fd_timeout(get_db_args[0], get_db_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) char *get_file_args[] = {"/bin/vconftool", "get", "file/", "-r", NULL}; ret = run_command_write_fd_timeout(get_file_args[0], get_file_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) } if (arg_dmesg && is_root) { @@ -211,29 +207,26 @@ int main(int argc, char *argv[]) char *dmesg_args[] = {"/bin/dmesg", "-T", NULL}; char *dmesg_env[] = {"TZ=UTC", NULL}; ret = run_command_write_fd_timeout(dmesg_args[0], dmesg_args, dmesg_env, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) } if (arg_dlog) { fprintf_fd(out_fd, "\n==== Log messages\n"); char *dlogutil_args[] = {"/bin/dlogutil", "-d", "-v", "threadtime", "-u", "16384", NULL}; ret = run_command_write_fd_timeout(dlogutil_args[0], dlogutil_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) } if (arg_journal) { fprintf_fd(out_fd, "\n==== Journal messages\n"); char *journalctl_args[] = {"/bin/journalctl", "-b", "-n", "1024", NULL}; ret = run_command_write_fd_timeout(journalctl_args[0], journalctl_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT); - if (ret < 0) - goto exit_close; + INFORM_IF_ERROR(ret) } -exit_close: if (arg_file) close(out_fd); exit: - return ret; + return exit_code; +#undef INFORM_IF_ERROR } -- 2.7.4 From 80be530942b9ce1c52367c646fd669622cd06da6 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Wed, 31 Oct 2018 14:04:09 +0100 Subject: [PATCH 10/16] Release 5.0.8 Changes: - Handle spaces in the crashed app pathname and in the maps file - Don't interrupt the dump_systemstate if any of subcommands return an error Change-Id: Ifc81df710c4441cbc63b42e719102e50f44b138b --- packaging/crash-worker.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index 7f1cf41..3537e86 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -10,7 +10,7 @@ Name: crash-worker Summary: Crash-manager -Version: 5.0.7 +Version: 5.0.8 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From 927ace8cc70a51f510d381da2208feaa53bc31e3 Mon Sep 17 00:00:00 2001 From: Mateusz Moscicki Date: Mon, 5 Nov 2018 11:39:40 +0100 Subject: [PATCH 11/16] crash-stack: Fix a /proc//maps file parsing Change-Id: If164f3bf5cfaf7eefc68d992d90ff4110b52ca04 --- src/crash-stack/crash-stack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crash-stack/crash-stack.c b/src/crash-stack/crash-stack.c index 61b876c..a348067 100644 --- a/src/crash-stack/crash-stack.c +++ b/src/crash-stack/crash-stack.c @@ -381,7 +381,7 @@ static struct addr_node *get_addr_list_from_maps(int fd) memset(path, 0, PATH_MAX+1); result = sscanf(linebuf, STR_FS(ADDR_LEN) STR_FS(PERM_LEN) - "%*s %*s %*s" + "%*s %*s %*s " STRWW_FS(PATH_MAX), addr, perm, path); if (result < 0) continue; -- 2.7.4 From dca1da43a76959fb8a6cc3bbd621ee08d2f72630 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Mon, 5 Nov 2018 14:02:41 +0100 Subject: [PATCH 12/16] Release 5.5.1 Version bump due to Tizen 5.0 M2 release & corresponding branching repo to additional tizen_5.0 line. This version contains one fix for /proc//maps parsing (not present in tizen_5.0). Change-Id: Ibbfebbd05b54bd81a0c55dafa357720ddfeb71cb --- packaging/crash-worker.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index 3537e86..1a952a8 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -10,7 +10,7 @@ Name: crash-worker Summary: Crash-manager -Version: 5.0.8 +Version: 5.5.1 Release: 1 Group: Framework/system License: Apache-2.0 and BSD -- 2.7.4 From ecbca2fe4b8722fd8dce6582abf46c35ae090883 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 3 Oct 2018 13:36:17 +0200 Subject: [PATCH 13/16] util: copy_bytes: Multiple changes - Use max possible buffer size for sendfile() The fewer we call sendfile() the better (performance) - Provide read()/write() fallback when sendfile will not work (eg. copying data from pipe to pipe) - Add third argument with number of copied bytes. This is to avoid mixing error code (negative) with transferred size. - Make API available for rest of codebase Change-Id: If80feb578b01fb715008a20e7c000c5ede9b62a1 --- CMakeLists.txt | 2 ++ src/shared/util.c | 68 ++++++++++++++++++++++++++++++++++++++++++++----------- src/shared/util.h | 2 ++ 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 96ecea7..9834ed5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,6 +3,8 @@ PROJECT(crash-worker C) SET(PREFIX ${CMAKE_INSTALL_PREFIX}) +ADD_DEFINITIONS(-D_FILE_OFFSET_BITS=64) + # Sub modules ADD_SUBDIRECTORY(src/crash-manager) diff --git a/src/shared/util.c b/src/shared/util.c index b83dbaa..d6fdd31 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -19,6 +19,7 @@ #endif #include #include +#include #include #include #include @@ -167,19 +168,60 @@ int write_fd(int fd, const void *buf, int len) return total; } -static int copy_bytes(int dfd, int sfd) +static int copy_bytes_sendfile(int destfd, int srcfd, off_t *ncopied) { - int res = 1; - ssize_t bytes_sent; - do - bytes_sent = sendfile(dfd, sfd, NULL, PIPE_BUF); - while (bytes_sent > 0); + ssize_t nsent; + off_t total = 0; - if (bytes_sent == -1) { - _E("sendfile() error: %m\n"); - res = -1; - } - return res; + do { + nsent = sendfile(destfd, srcfd, NULL, INT32_MAX); + if (nsent > 0) + total += nsent; + else if (nsent < 0 && errno == EAGAIN) + continue; + else + break; + } while (1); + + if (ncopied) + *ncopied = total; + + return nsent == -1 ? -1 : 0; +} + +static int copy_bytes_rw(int destfd, int srcfd, off_t *ncopied) +{ + ssize_t n; + char buf[4096]; + off_t total = 0; + + do { + n = read(srcfd, buf, sizeof buf); + if (n < 0) { + if (errno == EAGAIN || errno == EINTR) + continue; + _E("read: %m"); + return -1; + } + + int m = write_fd(destfd, buf, n); + if (m != n) { + _E("failed to write data to destination fd: %m"); + return -1; + } + total += m; + } while (n > 0); + + if (ncopied) + *ncopied = total; + + return 0; +} + +int copy_bytes(int destfd, int srcfd, off_t *ncopied) +{ + int r = copy_bytes_sendfile(destfd, srcfd, ncopied); + return r == 0 ? r : copy_bytes_rw(destfd, srcfd, ncopied); } int copy_file(char *src, char *dst) @@ -204,7 +246,7 @@ int copy_file(char *src, char *dst) return -1; } - res = copy_bytes(dfd, sfd); + res = copy_bytes(dfd, sfd, NULL); close(sfd); close(dfd); @@ -235,7 +277,7 @@ int dump_file_write_fd(char *src, int dfd) return -1; } - res = copy_bytes(dfd, sfd); + res = copy_bytes(dfd, sfd, NULL); close(sfd); return res; diff --git a/src/shared/util.h b/src/shared/util.h index 03de693..a010b7e 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -39,6 +39,8 @@ int wait_system_command(int pid); int write_fd(int fd, const void *buf, int len); +int copy_bytes(int destfd, int srcfd, off_t *ncopied); + int copy_file(char *src, char *dst); int move_file(char *src, char *dst); -- 2.7.4 From a4d1efddefa2114211d736479dd4813ecc1e8014 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 7 Nov 2018 12:00:18 +0100 Subject: [PATCH 14/16] util: Unify function calling convention to specify destination first This commit adjusts signature of copy_file(), move_file() and dump_file_write_fd() to specify destination first, source second. This in line with rest of codebase. Change-Id: I81285cda73d43ff2428ea5daf7ad27676e51ca8a Signed-off-by: Karol Lewandowski --- src/crash-manager/crash-manager.c | 2 +- src/dump_systemstate/dump_systemstate.c | 2 +- src/shared/util.c | 8 ++++---- src/shared/util.h | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index a02d59c..95cbf8b 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -623,7 +623,7 @@ static int get_sysassert_cs(struct crash_info *cinfo) return -1; } - if (move_file(cinfo->sysassert_cs_path, move_path) < 0) { + if (move_file(move_path, cinfo->sysassert_cs_path) < 0) { _E("Failed to move %s to %s", cinfo->sysassert_cs_path, move_path); return -1; diff --git a/src/dump_systemstate/dump_systemstate.c b/src/dump_systemstate/dump_systemstate.c index fb2ee75..d4c0e40 100644 --- a/src/dump_systemstate/dump_systemstate.c +++ b/src/dump_systemstate/dump_systemstate.c @@ -149,7 +149,7 @@ int main(int argc, char *argv[]) fsync(out_fd); fprintf_fd(out_fd, "\n%s(%s)\n", dump_item[i].title, dump_item[i].path); - ret = dump_file_write_fd((char *)dump_item[i].path, out_fd); + ret = dump_file_write_fd(out_fd, (char *)dump_item[i].path); INFORM_IF_ERROR(ret) } fprintf_fd(out_fd, "\n"); diff --git a/src/shared/util.c b/src/shared/util.c index d6fdd31..2367524 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -224,7 +224,7 @@ int copy_bytes(int destfd, int srcfd, off_t *ncopied) return r == 0 ? r : copy_bytes_rw(destfd, srcfd, ncopied); } -int copy_file(char *src, char *dst) +int copy_file(char *dst, char *src) { int res; int sfd; @@ -253,16 +253,16 @@ int copy_file(char *src, char *dst) return res; } -int move_file(char *src, char *dst) +int move_file(char *dst, char *src) { - if (copy_file(src, dst) < 0) + if (copy_file(dst, src) < 0) return -1; if (unlink(src) < 0) return -1; return 1; } -int dump_file_write_fd(char *src, int dfd) +int dump_file_write_fd(int dfd, char *src) { int res; int sfd; diff --git a/src/shared/util.h b/src/shared/util.h index a010b7e..dfe40bf 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -41,11 +41,11 @@ int write_fd(int fd, const void *buf, int len); int copy_bytes(int destfd, int srcfd, off_t *ncopied); -int copy_file(char *src, char *dst); +int copy_file(char *dst, char *src); -int move_file(char *src, char *dst); +int move_file(char *dst, char *src); -int dump_file_write_fd(char *src, int dfd); +int dump_file_write_fd(int dfd, char *src); int run_command_write_fd_timeout(char *path, char *args[], char *env[], int dfd, char *buff, int size, int timeout); -- 2.7.4 From 88120bf4a98462b41b5309017fc2ac0e332cb945 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 3 Oct 2018 13:40:13 +0200 Subject: [PATCH 15/16] packaging: Drop unused variables from cmake invocation Ideally, crash-worker codebase should not know about any of TZ_ vars - as all needed paths should be specified explicitly from the .spec. Change-Id: Id80fe0ae5f93940ce49bbe11903723a00f0261f0 --- packaging/crash-worker.spec | 2 -- 1 file changed, 2 deletions(-) diff --git a/packaging/crash-worker.spec b/packaging/crash-worker.spec index 1a952a8..024821a 100644 --- a/packaging/crash-worker.spec +++ b/packaging/crash-worker.spec @@ -102,8 +102,6 @@ export CFLAGS+=" -Werror" -DARCH=%{ARCH} \ -DARCH_BIT=%{ARCH_BIT} \ -DCRASH_MANAGER_CONFIG_PATH=%{_sysconfdir}/crash-manager.conf \ - -DTZ_SYS_ETC=%{TZ_SYS_ETC} \ - -DTZ_SYS_BIN=%{TZ_SYS_BIN} \ -DCRASH_ROOT_PATH=%{crash_root_path} \ -DCRASH_PATH=%{crash_path} \ -DCRASH_TEMP=%{crash_temp} \ -- 2.7.4 From 236600bdad00d1f53b4221e0cac4a47bc64d1645 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Wed, 3 Oct 2018 13:59:18 +0200 Subject: [PATCH 16/16] Define _GNU_SOURCE once for whole codebase There is no need to repeat this in every file. Change-Id: Ia788840d819b8d72122296a5e5d64aa2a7c3cb40 --- CMakeLists.txt | 2 +- src/crash-manager/crash-manager.c | 2 -- src/crash-manager/dbus_notify.c | 1 - src/crash-stack/CMakeLists.txt | 2 +- src/crash-stack/crash-stack.c | 1 - src/shared/util.c | 3 --- src/sys-assert/arm/backtrace.c | 2 +- src/sys-assert/arm_64/backtrace.c | 2 +- src/sys-assert/sys-assert.c | 1 - src/sys-assert/x86/backtrace.c | 2 +- src/sys-assert/x86/context.c | 2 +- 11 files changed, 6 insertions(+), 14 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9834ed5..9f3222d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3,7 +3,7 @@ PROJECT(crash-worker C) SET(PREFIX ${CMAKE_INSTALL_PREFIX}) -ADD_DEFINITIONS(-D_FILE_OFFSET_BITS=64) +ADD_DEFINITIONS(-D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE=1) # Sub modules ADD_SUBDIRECTORY(src/crash-manager) diff --git a/src/crash-manager/crash-manager.c b/src/crash-manager/crash-manager.c index 95cbf8b..c48e29a 100644 --- a/src/crash-manager/crash-manager.c +++ b/src/crash-manager/crash-manager.c @@ -16,8 +16,6 @@ * limitations under the License. */ -#define _GNU_SOURCE - #include #include #include diff --git a/src/crash-manager/dbus_notify.c b/src/crash-manager/dbus_notify.c index 6a8c5d8..cf78f6a 100644 --- a/src/crash-manager/dbus_notify.c +++ b/src/crash-manager/dbus_notify.c @@ -16,7 +16,6 @@ * Author: Mateusz Moscicki */ -#define _GNU_SOURCE #include #include #include diff --git a/src/crash-stack/CMakeLists.txt b/src/crash-stack/CMakeLists.txt index fd26fac..f996da9 100644 --- a/src/crash-stack/CMakeLists.txt +++ b/src/crash-stack/CMakeLists.txt @@ -21,7 +21,7 @@ else() set(CRASH_STACK_SRCS ${CRASH_STACK_SRCS} crash-stack-stub.c) endif() -set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -fPIE -D_GNU_SOURCE=1") +set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -fPIE") set(CMAKE_EXE_LINKER_FLAGS "-Wl,--as-needed -pie") # Binary diff --git a/src/crash-stack/crash-stack.c b/src/crash-stack/crash-stack.c index a348067..00e2f3f 100644 --- a/src/crash-stack/crash-stack.c +++ b/src/crash-stack/crash-stack.c @@ -17,7 +17,6 @@ * Łukasz Stelmach * Rafał Pietruch */ -#define _GNU_SOURCE 1 /** * @file crash-stack.c diff --git a/src/shared/util.c b/src/shared/util.c index 2367524..775d6be 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -14,9 +14,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#ifndef _GNU_SOURCE -#define _GNU_SOURCE -#endif #include #include #include diff --git a/src/sys-assert/arm/backtrace.c b/src/sys-assert/arm/backtrace.c index 7520bba..2d1241a 100644 --- a/src/sys-assert/arm/backtrace.c +++ b/src/sys-assert/arm/backtrace.c @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#define _GNU_SOURCE + #include #include #include diff --git a/src/sys-assert/arm_64/backtrace.c b/src/sys-assert/arm_64/backtrace.c index 3c8f560..21a10d4 100644 --- a/src/sys-assert/arm_64/backtrace.c +++ b/src/sys-assert/arm_64/backtrace.c @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#define _GNU_SOURCE + #include #include #include diff --git a/src/sys-assert/sys-assert.c b/src/sys-assert/sys-assert.c index 75c7c3b..5af44ff 100644 --- a/src/sys-assert/sys-assert.c +++ b/src/sys-assert/sys-assert.c @@ -15,7 +15,6 @@ * limitations under the License. */ -#define _GNU_SOURCE #include #include #include diff --git a/src/sys-assert/x86/backtrace.c b/src/sys-assert/x86/backtrace.c index 0453bbb..4c07d86 100644 --- a/src/sys-assert/x86/backtrace.c +++ b/src/sys-assert/x86/backtrace.c @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#define _GNU_SOURCE + #include #include #include diff --git a/src/sys-assert/x86/context.c b/src/sys-assert/x86/context.c index 5644d15..313ad86 100644 --- a/src/sys-assert/x86/context.c +++ b/src/sys-assert/x86/context.c @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#define _GNU_SOURCE + #include #include #include -- 2.7.4