From: Andre Noll Date: Fri, 7 Jun 2013 19:34:51 +0000 (+0200) Subject: Merge branch 't/gui_improvements' X-Git-Tag: v0.4.13~33 X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=1775d4d4147730e79e48aa941aee88580b8beb08;hp=e065f3eec9be99d82da855bf3da38d8e86752fef Merge branch 't/gui_improvements' Was cooking for more than one month. bf1831 string: Speed up xvasprintf(). bcc083 string: Add discard feature for for_each_line(). e3868d string: Simplify return value of for_each_line(). d1f0f0 string: Simplify for_each_line(). 23b121 string: Clean up for_each_line() and related functions. 6256ed string: Replace the for_each_line_modes enum by a bitmap. 14e689 gui: Don't sleep before executing the status command. e90c6c gui: Speed up add_spaces(). 20e2c6 gui: Rename do_exit(). ef0508 gui: Remove superfluous cmd_died. d7562b gui: Check stdin for readability. a44fa6 gui: Discard overlong input lines. Conflicts: gui.c --- diff --git a/NEWS b/NEWS index 56f84753..a93fb2e3 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,7 @@ - Scheduler improvements and fixes. - The obsolete gettimeofday() function has been replaced by clock_gettime() on systems which support it. + - Speed and usability improvements for para_gui. ----------------------------------------- 0.4.12 (2012-12-20) "volatile relativity" diff --git a/configure.ac b/configure.ac index 135a0bd9..12959c81 100644 --- a/configure.ac +++ b/configure.ac @@ -153,7 +153,7 @@ client_errlist_objs="client net string fd sched stdin stdout time sideband client_ldflags="" gui_cmdline_objs="add_cmdline(gui)" -gui_errlist_objs="exec signal string stat ringbuffer fd gui gui_theme" +gui_errlist_objs="exec signal string stat ringbuffer fd gui gui_theme time" gui_objs="$gui_cmdline_objs $gui_errlist_objs" play_errlist_objs="play fd sched ggo buffer_tree time string net afh_recv afh_common diff --git a/gui.c b/gui.c index 631e7cc5..096beb93 100644 --- a/gui.c +++ b/gui.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "gui.cmdline.h" #include "para.h" @@ -51,7 +52,7 @@ static struct ringbuffer *bot_win_rb; static unsigned scroll_position; -static int cmd_died, curses_active; +static int curses_active; static pid_t cmd_pid; static int command_fds[2]; @@ -288,14 +289,20 @@ static char *configfile_exists(void) return file_exists(tmp)? tmp: NULL; } -/* - * print num spaces to curses window - */ +/* Print given number of spaces to curses window. */ static void add_spaces(WINDOW* win, unsigned int num) { - while (num > 0) { - num--; - waddstr(win, " "); + char space[] = " "; + unsigned sz = sizeof(space); + + while (num >= sz) { + waddstr(win, space); + num -= sz; + } + if (num > 0) { + assert(num < sz); + space[num] = '\0'; + waddstr(win, space); } } @@ -555,7 +562,8 @@ static void setup_signal_handling(void) para_sigaction(SIGHUP, SIG_IGN); } -__noreturn static void do_exit(int ret) +/* kill every process in the process group and exit */ +__noreturn static void kill_pg_and_die(int ret) { para_sigaction(SIGTERM, SIG_IGN); kill(0, SIGTERM); @@ -574,7 +582,7 @@ static void shutdown_curses(void) __noreturn static void finish(int ret) { shutdown_curses(); - do_exit(ret); + kill_pg_and_die(ret); } /* @@ -589,7 +597,7 @@ __noreturn __printf_2_3 static void msg_n_exit(int ret, const char* fmt, ...) va_start(argp, fmt); vfprintf(outfd, fmt, argp); va_end(argp); - do_exit(ret); + kill_pg_and_die(ret); } static void print_welcome(void) @@ -856,10 +864,8 @@ reap_next_child: ret = para_reap_child(&pid); if (ret <= 0) return; - if (pid == cmd_pid) { + if (pid == cmd_pid) cmd_pid = 0; - cmd_died = 1; - } goto reap_next_child; } @@ -947,33 +953,36 @@ static void handle_signal(int sig) } } -static int open_stat_pipe(void) +static void status_pre_select(fd_set *rfds, int *max_fileno, struct timeval *tv) { - static int init = 1; + static struct timeval next_exec, atm, diff; int ret, fds[3] = {0, 1, 0}; pid_t pid; - if (init) - init = 0; - else - /* - * Sleep a bit to avoid a busy loop. As the call to sleep() may - * be interrupted by SIGCHLD, we simply wait until the call - * succeeds. - */ - while (sleep(2)) - ; /* nothing */ + if (stat_pipe >= 0) + goto success; + /* Avoid busy loop */ + gettimeofday(&atm, NULL); + if (tv_diff(&next_exec, &atm, &diff) > 0) { + if (tv_diff(&diff, tv, NULL) < 0) + *tv = diff; + return; + } + next_exec.tv_sec = atm.tv_sec + 2; ret = para_exec_cmdline_pid(&pid, conf.stat_cmd_arg, fds); if (ret < 0) - return ret; + return; ret = mark_fd_nonblocking(fds[1]); - if (ret >= 0) - return fds[1]; - close(fds[1]); - return ret; + if (ret < 0) { + close(fds[1]); + return; + } + stat_pipe = fds[1]; +success: + para_fd_set(stat_pipe, rfds, max_fileno); } -#define COMMAND_BUF_SIZE 4096 +#define COMMAND_BUF_SIZE 32768 /* * This is the core select loop. Besides the (internal) signal @@ -995,6 +1004,7 @@ static int do_select(int mode) char command_buf[2][COMMAND_BUF_SIZE] = {"", ""}; int cbo[2] = {0, 0}; /* command buf offsets */ struct timeval tv; + unsigned flags[2] = {0, 0}; /* for for_each_line() */ repeat: tv.tv_sec = conf.timeout_arg / 1000; @@ -1002,10 +1012,7 @@ repeat: // ret = refresh_status(); FD_ZERO(&rfds); max_fileno = 0; - if (stat_pipe < 0) - stat_pipe = open_stat_pipe(); - if (stat_pipe >= 0) - para_fd_set(stat_pipe, &rfds, &max_fileno); + status_pre_select(&rfds, &max_fileno, &tv); /* signal pipe */ para_fd_set(signal_pipe, &rfds, &max_fileno); /* command pipe only for COMMAND_MODE */ @@ -1015,6 +1022,8 @@ repeat: if (command_fds[1] >= 0) para_fd_set(command_fds[1], &rfds, &max_fileno); } + if (mode == GETCH_MODE || mode == COMMAND_MODE) + para_fd_set(STDIN_FILENO, &rfds, &max_fileno); ret = para_select(max_fileno + 1, &rfds, NULL, &tv); if (ret <= 0) goto check_return; /* skip fd checks */ @@ -1033,18 +1042,26 @@ repeat: COMMAND_BUF_SIZE - 1 - cbo[i], &rfds, &sz); cbo[i] += sz; sz = cbo[i]; - cbo[i] = for_each_line(command_buf[i], cbo[i], + cbo[i] = for_each_line(flags[i], command_buf[i], cbo[i], add_output_line, &i); - if (sz != cbo[i]) + if (sz != cbo[i]) { /* at least one line found */ wrefresh(bot.win); + flags[i] = 0; + } if (ret < 0) { PARA_NOTICE_LOG("closing command fd %d: %s", i, para_strerror(-ret)); close(command_fds[i]); command_fds[i] = -1; + flags[i] = 0; if (command_fds[!i] < 0) /* both fds closed */ return 0; } + if (cbo[i] == COMMAND_BUF_SIZE - 1) { + PARA_NOTICE_LOG("discarding overlong line"); + cbo[i] = 0; + flags[i] = FELF_DISCARD_FIRST; + } } } ret = read_stat_pipe(&rfds); @@ -1082,10 +1099,8 @@ check_return: return ret; break; case EXTERNAL_MODE: - if (cmd_died) { - cmd_died = 0; + if (cmd_pid == 0) return 0; - } } goto repeat; } @@ -1159,7 +1174,6 @@ static void external_cmd(char *cmd) shutdown_curses(); if (para_exec_cmdline_pid(&cmd_pid, cmd, fds) < 0) return; - cmd_died = 0; do_select(EXTERNAL_MODE); init_curses(); } diff --git a/mood.c b/mood.c index 5d2d38b4..e06382e8 100644 --- a/mood.c +++ b/mood.c @@ -386,7 +386,7 @@ static int load_mood(const struct osl_row *mood_row, struct mood **m) if (!*mood_name) return -E_DUMMY_ROW; mlpd.m = alloc_new_mood(mood_name); - ret = for_each_line_ro(mood_def.data, mood_def.size, + ret = for_each_line(FELF_READ_ONLY, mood_def.data, mood_def.size, parse_mood_line, &mlpd); osl_close_disk_object(&mood_def); if (ret < 0) { @@ -418,7 +418,7 @@ static int check_mood(struct osl_row *mood_row, void *data) ret = para_printf(pb, "checking mood %s...\n", mood_name); if (ret < 0) goto out; - ret = for_each_line_ro(mood_def.data, mood_def.size, + ret = for_each_line(FELF_READ_ONLY, mood_def.data, mood_def.size, parse_mood_line, &mlpd); if (ret < 0) para_printf(pb, "%s line %u: %s\n", mood_name, mlpd.line_num, diff --git a/playlist.c b/playlist.c index 60e27e6e..c822e080 100644 --- a/playlist.c +++ b/playlist.c @@ -70,8 +70,8 @@ static int load_playlist(struct osl_row *row, void *data) if (ret < 0) goto err; playlist->length = 0; - ret = for_each_line_ro(playlist_def.data, playlist_def.size, - add_playlist_entry, playlist); + ret = for_each_line(FELF_READ_ONLY, playlist_def.data, + playlist_def.size, add_playlist_entry, playlist); osl_close_disk_object(&playlist_def); if (ret < 0) goto err; @@ -114,8 +114,8 @@ static int check_playlist(struct osl_row *row, void *data) ret = para_printf(pb, "checking playlist %s...\n", playlist_name); if (ret < 0) return ret; - ret = for_each_line_ro(playlist_def.data, playlist_def.size, - check_playlist_path, pb); + ret = for_each_line(FELF_READ_ONLY, playlist_def.data, + playlist_def.size, check_playlist_path, pb); } osl_close_disk_object(&playlist_def); return ret; @@ -219,8 +219,8 @@ static int handle_audio_file_event(enum afs_events event, void *data) ret = pl_get_def_by_name(current_playlist.name, &playlist_def); if (ret < 0) return ret; - ret = for_each_line_ro(playlist_def.data, playlist_def.size, - search_path, new_path); + ret = for_each_line(FELF_READ_ONLY, playlist_def.data, + playlist_def.size, search_path, new_path); osl_close_disk_object(&playlist_def); is_admissible = (ret < 0); if (was_admissible && is_admissible) diff --git a/string.c b/string.c index dfcfa2cd..38e25b09 100644 --- a/string.c +++ b/string.c @@ -142,15 +142,18 @@ __must_check __malloc char *para_strdup(const char *s) __printf_2_0 unsigned xvasprintf(char **result, const char *fmt, va_list ap) { int ret; - size_t size; + size_t size = 150; va_list aq; + *result = para_malloc(size + 1); va_copy(aq, ap); - ret = vsnprintf(NULL, 0, fmt, aq); + ret = vsnprintf(*result, size, fmt, aq); va_end(aq); assert(ret >= 0); + if (ret < size) /* OK */ + return ret; size = ret + 1; - *result = para_malloc(size); + *result = para_realloc(*result, size); va_copy(aq, ap); ret = vsnprintf(*result, size, fmt, aq); va_end(aq); @@ -356,19 +359,35 @@ __malloc char *para_hostname(void) } /** - * Used to distinguish between read-only and read-write mode. + * Call a custom function for each complete line. + * + * \param flags Any combination of flags defined in \ref for_each_line_flags. + * \param buf The buffer containing data separated by newlines. + * \param size The number of bytes in \a buf. + * \param line_handler The custom function. + * \param private_data Pointer passed to \a line_handler. + * + * For each complete line in \p buf, \p line_handler is called. The first + * argument to \p line_handler is (a copy of) the current line, and \p + * private_data is passed as the second argument. If the \p FELF_READ_ONLY + * flag is unset, a pointer into \a buf is passed to the line handler, + * otherwise a pointer to a copy of the current line is passed instead. This + * copy is freed immediately after the line handler returns. + * + * The function returns if \p line_handler returns a negative value or no more + * lines are in the buffer. The rest of the buffer (last chunk containing an + * incomplete line) is moved to the beginning of the buffer if FELF_READ_ONLY is + * unset. * - * \sa for_each_line(), for_each_line_ro(). + * \return On success this function returns the number of bytes not handled to + * \p line_handler. The only possible error is a negative return value from the + * line handler. In this case processing stops and the return value of the line + * handler is returned to indicate failure. + * + * \sa \ref for_each_line_flags. */ -enum for_each_line_modes{ - /** Activate read-only mode. */ - LINE_MODE_RO, - /** Activate read-write mode. */ - LINE_MODE_RW -}; - -static int for_each_complete_line(enum for_each_line_modes mode, char *buf, - size_t size, line_handler_t *line_handler, void *private_data) +int for_each_line(unsigned flags, char *buf, size_t size, + line_handler_t *line_handler, void *private_data) { char *start = buf, *end; int ret, i, num_lines = 0; @@ -389,85 +408,29 @@ static int for_each_complete_line(enum for_each_line_modes mode, char *buf, } else end = next_cr; num_lines++; - if (!line_handler) { - start = ++end; - continue; - } - if (mode == LINE_MODE_RO) { - size_t s = end - start; - char *b = para_malloc(s + 1); - memcpy(b, start, s); - b[s] = '\0'; -// PARA_NOTICE_LOG("b: %s, start: %s\n", b, start); - ret = line_handler(b, private_data); - free(b); - } else { - *end = '\0'; - ret = line_handler(start, private_data); + if (!(flags & FELF_DISCARD_FIRST) || start != buf) { + if (flags & FELF_READ_ONLY) { + size_t s = end - start; + char *b = para_malloc(s + 1); + memcpy(b, start, s); + b[s] = '\0'; + ret = line_handler(b, private_data); + free(b); + } else { + *end = '\0'; + ret = line_handler(start, private_data); + } + if (ret < 0) + return ret; } - if (ret < 0) - return ret; start = ++end; } - if (!line_handler || mode == LINE_MODE_RO) - return num_lines; i = buf + size - start; - if (i && i != size) + if (i && i != size && !(flags & FELF_READ_ONLY)) memmove(buf, start, i); return i; } -/** - * Call a custom function for each complete line. - * - * \param buf The buffer containing data separated by newlines. - * \param size The number of bytes in \a buf. - * \param line_handler The custom function. - * \param private_data Pointer passed to \a line_handler. - * - * If \p line_handler is \p NULL, the function returns the number of complete - * lines in \p buf. Otherwise, \p line_handler is called for each complete - * line in \p buf. The first argument to \p line_handler is the current line, - * and \p private_data is passed as the second argument. The function returns - * if \p line_handler returns a negative value or no more lines are in the - * buffer. The rest of the buffer (last chunk containing an incomplete line) - * is moved to the beginning of the buffer. - * - * \return If \p line_handler is not \p NULL, this function returns the number - * of bytes not handled to \p line_handler on success, or the negative return - * value of the \p line_handler on errors. - * - * \sa for_each_line_ro(). - */ -int for_each_line(char *buf, size_t size, line_handler_t *line_handler, - void *private_data) -{ - return for_each_complete_line(LINE_MODE_RW, buf, size, line_handler, - private_data); -} - -/** - * Call a custom function for each complete line. - * - * \param buf Same meaning as in \p for_each_line(). - * \param size Same meaning as in \p for_each_line(). - * \param line_handler Same meaning as in \p for_each_line(). - * \param private_data Same meaning as in \p for_each_line(). - * - * This function behaves like \p for_each_line(), but \a buf is left unchanged. - * - * \return On success, the function returns the number of complete lines in \p - * buf, otherwise the (negative) return value of \p line_handler is returned. - * - * \sa for_each_line(). - */ -int for_each_line_ro(char *buf, size_t size, line_handler_t *line_handler, - void *private_data) -{ - return for_each_complete_line(LINE_MODE_RO, buf, size, line_handler, - private_data); -} - /** Return the hex characters of the lower 4 bits. */ #define hex(a) (hexchar[(a) & 15]) diff --git a/string.h b/string.h index d45c9199..935c7d45 100644 --- a/string.h +++ b/string.h @@ -34,6 +34,23 @@ struct para_buffer { void *private_data; }; +/** + * Controls the behavior of for_each_line(). + * + * \sa for_each_line(). + */ +enum for_each_line_flags { + /** Activate read-only mode. */ + FELF_READ_ONLY = 1 << 0, + /** Don't call line handler for the first input line. */ + FELF_DISCARD_FIRST = 1 << 1, +}; + +/** Used for \ref for_each_line(). */ +typedef int line_handler_t(char *, void *); +int for_each_line(unsigned flags, char *buf, size_t size, + line_handler_t *line_handler, void *private_data); + /** * Write the contents of a status item to a para_buffer. * @@ -72,12 +89,6 @@ __must_check __malloc char *para_logname(void); __must_check __malloc char *para_homedir(void); __malloc char *para_hostname(void); __printf_2_3 int para_printf(struct para_buffer *b, const char *fmt, ...); -/** Used for for_each_line() and for_each_line_ro(). */ -typedef int line_handler_t(char *, void *); -int for_each_line(char *buf, size_t size, line_handler_t *line_handler, - void *private_data); -int for_each_line_ro(char *buf, size_t size, line_handler_t *line_handler, - void *private_data); int para_atoi64(const char *str, int64_t *result); int para_atoi32(const char *str, int32_t *value); int get_loglevel_by_name(const char *txt);