From 42515308e72e4ea6f6b49508c1ba286263dded64 Mon Sep 17 00:00:00 2001 From: hwangcc Date: Tue, 24 Mar 2015 20:57:06 +0800 Subject: [PATCH] Add a safe wrapper for write and fix some warnings 1. Add a function writeall and make swrite wrap that function. Use either writeall or swrite, depending on whether we want to exit on errors or not. 2. Fix warnings when compiling with a higher optimisation level. (CFLAGS ?= -pipe -O3 -march=native -mtune=native -freorder-blocks-and-partition) Signed-off-by: hwangcc --- i3-dump-log/main.c | 11 +++-------- i3-nagbar/main.c | 4 +++- i3bar/src/child.c | 17 ++++++++++++++--- i3bar/src/ipc.c | 13 +------------ include/libi3.h | 14 ++++++++++++++ libi3/ipc_send_message.c | 30 ++++-------------------------- libi3/safewrappers.c | 29 +++++++++++++++++++++++++++++ src/click.c | 3 +++ src/config_parser.c | 13 ++++--------- src/load_layout.c | 2 +- src/sighandler.c | 10 ++++++++-- src/util.c | 24 ++++++------------------ 12 files changed, 90 insertions(+), 80 deletions(-) diff --git a/i3-dump-log/main.c b/i3-dump-log/main.c index 86b39338..1b0d593c 100644 --- a/i3-dump-log/main.c +++ b/i3-dump-log/main.c @@ -43,8 +43,7 @@ static int check_for_wrap(void) { * of the log. */ wrap_count = header->wrap_count; const int len = (logbuffer + header->offset_last_wrap) - walk; - if (write(STDOUT_FILENO, walk, len) != len) - err(EXIT_FAILURE, "write()"); + swrite(STDOUT_FILENO, walk, len); walk = logbuffer + sizeof(i3_shmlog_header); return 1; } @@ -52,12 +51,8 @@ static int check_for_wrap(void) { static void print_till_end(void) { check_for_wrap(); const int len = (logbuffer + header->offset_next_write) - walk; - const int n = write(STDOUT_FILENO, walk, len); - if (len != n) - err(EXIT_FAILURE, "write()"); - if (n > 0) { - walk += n; - } + swrite(STDOUT_FILENO, walk, len); + walk += len; } int main(int argc, char *argv[]) { diff --git a/i3-nagbar/main.c b/i3-nagbar/main.c index b501ff6a..91de317f 100644 --- a/i3-nagbar/main.c +++ b/i3-nagbar/main.c @@ -164,7 +164,9 @@ static void handle_button_release(xcb_connection_t *conn, xcb_button_release_eve char *link_path; char *exe_path = get_exe_path(argv0); sasprintf(&link_path, "%s.nagbar_cmd", script_path); - symlink(exe_path, link_path); + if (symlink(exe_path, link_path) == -1) { + err(EXIT_FAILURE, "Failed to symlink %s to %s", link_path, exe_path); + } char *terminal_cmd; sasprintf(&terminal_cmd, "i3-sensible-terminal -e %s", link_path); diff --git a/i3bar/src/child.c b/i3bar/src/child.c index 402e6351..818ce784 100644 --- a/i3bar/src/child.c +++ b/i3bar/src/child.c @@ -103,7 +103,7 @@ __attribute__((format(printf, 1, 2))) static void set_statusline_error(const cha char *message; va_list args; va_start(args, format); - vasprintf(&message, format, args); + (void)vasprintf(&message, format, args); struct status_block *err_block = scalloc(sizeof(struct status_block)); err_block->full_text = i3string_from_utf8("Error: "); @@ -455,11 +455,22 @@ void child_write_output(void) { if (child.click_events) { const unsigned char *output; size_t size; + ssize_t n; yajl_gen_get_buf(gen, &output, &size); - write(child_stdin, output, size); - write(child_stdin, "\n", 1); + + n = writeall(child_stdin, output, size); + if (n != -1) + n = writeall(child_stdin, "\n", 1); + yajl_gen_clear(gen); + + if (n == -1) { + child.click_events = false; + kill_child(); + set_statusline_error("child_write_output failed"); + draw_bars(false); + } } } diff --git a/i3bar/src/ipc.c b/i3bar/src/ipc.c index c6e67eb6..edc9d73f 100644 --- a/i3bar/src/ipc.c +++ b/i3bar/src/ipc.c @@ -296,18 +296,7 @@ int i3_send_msg(uint32_t type, const char *payload) { if (payload != NULL) strncpy(walk, payload, len); - uint32_t written = 0; - - while (to_write > 0) { - int n = write(i3_connection->fd, buffer + written, to_write); - if (n == -1) { - ELOG("write() failed: %s\n", strerror(errno)); - exit(EXIT_FAILURE); - } - - to_write -= n; - written += n; - } + swrite(i3_connection->fd, buffer, to_write); FREE(buffer); diff --git a/include/libi3.h b/include/libi3.h index c8d2e956..3a125827 100644 --- a/include/libi3.h +++ b/include/libi3.h @@ -134,6 +134,20 @@ char *sstrdup(const char *str); */ int sasprintf(char **strp, const char *fmt, ...); +/** + * Wrapper around correct write which returns -1 (meaning that + * write failed) or count (meaning that all bytes were written) + * + */ +ssize_t writeall(int fd, const void *buf, size_t count); + +/** + * Safe-wrapper around writeall which exits if it returns -1 (meaning that + * write failed) + * + */ +ssize_t swrite(int fd, const void *buf, size_t count); + /** * Build an i3String from an UTF-8 encoded string. * Returns the newly-allocated i3String. diff --git a/libi3/ipc_send_message.c b/libi3/ipc_send_message.c index 28cb8359..80709ed3 100644 --- a/libi3/ipc_send_message.c +++ b/libi3/ipc_send_message.c @@ -32,33 +32,11 @@ int ipc_send_message(int sockfd, const uint32_t message_size, .size = message_size, .type = message_type}; - size_t sent_bytes = 0; - int n = 0; + if (writeall(sockfd, ((void *)&header), sizeof(i3_ipc_header_t)) == -1) + return -1; - /* This first loop is basically unnecessary. No operating system has - * buffers which cannot fit 14 bytes into them, so the write() will only be - * called once. */ - while (sent_bytes < sizeof(i3_ipc_header_t)) { - if ((n = write(sockfd, ((void *)&header) + sent_bytes, sizeof(i3_ipc_header_t) - sent_bytes)) == -1) { - if (errno == EAGAIN) - continue; - return -1; - } - - sent_bytes += n; - } - - sent_bytes = 0; - - while (sent_bytes < message_size) { - if ((n = write(sockfd, payload + sent_bytes, message_size - sent_bytes)) == -1) { - if (errno == EAGAIN) - continue; - return -1; - } - - sent_bytes += n; - } + if (writeall(sockfd, payload, message_size) == -1) + return -1; return 0; } diff --git a/libi3/safewrappers.c b/libi3/safewrappers.c index cf634ad4..6acf3109 100644 --- a/libi3/safewrappers.c +++ b/libi3/safewrappers.c @@ -8,8 +8,10 @@ #include #include #include +#include #include #include +#include #include "libi3.h" @@ -56,3 +58,30 @@ int sasprintf(char **strp, const char *fmt, ...) { va_end(args); return result; } + +ssize_t writeall(int fd, const void *buf, size_t count) { + int written = 0; + ssize_t n = 0; + + while (written < count) { + n = write(fd, buf + written, count - written); + if (n == -1) { + if (errno == EINTR || errno == EAGAIN) + continue; + return n; + } + written += n; + } + + return written; +} + +ssize_t swrite(int fd, const void *buf, size_t count) { + ssize_t n; + + n = writeall(fd, buf, count); + if (n == -1) + err(EXIT_FAILURE, "Failed to write %d", fd); + else + return n; +} diff --git a/src/click.c b/src/click.c index 55e7147c..10abc057 100644 --- a/src/click.c +++ b/src/click.c @@ -46,6 +46,9 @@ static bool tiling_resize_for_border(Con *con, border_t border, xcb_button_press case BORDER_BOTTOM: search_direction = D_DOWN; break; + default: + assert(false); + break; } bool res = resize_find_tiling_participants(&first, &second, search_direction); diff --git a/src/config_parser.c b/src/config_parser.c index b229b445..eef03cae 100644 --- a/src/config_parser.c +++ b/src/config_parser.c @@ -778,14 +778,9 @@ static char *migrate_config(char *input, off_t size) { /* write the whole config file to the pipe, the script will read everything * immediately */ - int written = 0; - int ret; - while (written < size) { - if ((ret = write(writepipe[1], input + written, size - written)) < 0) { - warn("Could not write to pipe"); - return NULL; - } - written += ret; + if (writeall(writepipe[1], input, size) == -1) { + warn("Could not write to pipe"); + return NULL; } close(writepipe[1]); @@ -795,7 +790,7 @@ static char *migrate_config(char *input, off_t size) { /* read the script’s output */ int conv_size = 65535; char *converted = malloc(conv_size); - int read_bytes = 0; + int read_bytes = 0, ret; do { if (read_bytes == conv_size) { conv_size += 65535; diff --git a/src/load_layout.c b/src/load_layout.c index ccd71c37..c4d39fce 100644 --- a/src/load_layout.c +++ b/src/load_layout.c @@ -105,7 +105,7 @@ static int json_end_map(void *ctx) { int cnt = 1; while (workspace != NULL) { FREE(json_node->name); - asprintf(&(json_node->name), "%s_%d", base, cnt++); + sasprintf(&(json_node->name), "%s_%d", base, cnt++); workspace = NULL; TAILQ_FOREACH(output, &(croot->nodes_head), nodes) GREP_FIRST(workspace, output_get_content(output), !strcasecmp(child->name, json_node->name)); diff --git a/src/sighandler.c b/src/sighandler.c index 546b73d9..e971f6bd 100644 --- a/src/sighandler.c +++ b/src/sighandler.c @@ -70,8 +70,14 @@ static int backtrace(void) { int stdin_pipe[2], stdout_pipe[2]; - pipe(stdin_pipe); - pipe(stdout_pipe); + if (pipe(stdin_pipe) == -1) { + ELOG("Failed to init stdin_pipe\n"); + return -1; + } + if (pipe(stdout_pipe) == -1) { + ELOG("Failed to init stdout_pipe\n"); + return -1; + } /* close standard streams in case i3 is started from a terminal; gdb * needs to run without controlling terminal for it to work properly in diff --git a/src/util.c b/src/util.c index c5c22ba8..5760ae72 100644 --- a/src/util.c +++ b/src/util.c @@ -265,25 +265,13 @@ char *store_restart_layout(void) { return NULL; } - size_t written = 0; - while (written < length) { - int n = write(fd, payload + written, length - written); - /* TODO: correct error-handling */ - if (n == -1) { - perror("write()"); - free(filename); - close(fd); - return NULL; - } - if (n == 0) { - DLOG("write == 0?\n"); - free(filename); - close(fd); - return NULL; - } - written += n; - DLOG("written: %zd of %zd\n", written, length); + if (writeall(fd, payload, length) == -1) { + ELOG("Could not write restart layout to \"%s\", layout will be lost: %s\n", filename, strerror(errno)); + free(filename); + close(fd); + return NULL; } + close(fd); if (length > 0) { -- 2.39.2