[pacman-dev] [PATCH] cache terminal column width and handle SIGWINCH
Instead of doing ioctl each time, cache the result. Handle SIGWINCH to we know the size of the terminal has changed (handle by clearing the cache). --- Inspired by systemd. This is to set things up for paging, as paging would require caching this value. Once the pager is up, there's no way to poll for columns. src/pacman/pacman.c | 10 ++++++++++ src/pacman/util.c | 20 ++++++++++++++++---- src/pacman/util.h | 1 + 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index df73bcf..1643a63 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -1044,6 +1044,16 @@ int main(int argc, char *argv[]) } } + new_action.sa_handler = columns_reset; + sigemptyset(&new_action.sa_mask); + new_action.sa_flags = 0; + + sigaction(SIGWINCH, NULL, &old_action); + if(old_action.sa_handler != SIG_IGN) { + sigaction(SIGWINCH, &new_action, NULL); + } + + /* i18n init */ #if defined(ENABLE_NLS) localize(); diff --git a/src/pacman/util.c b/src/pacman/util.c index 23eea98..fec14b7 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -46,6 +46,7 @@ #include "conf.h" #include "callback.h" +static volatile unsigned cached_columns = 0; struct table_cell_t { char *label; @@ -155,12 +156,21 @@ static int flush_term_input(int fd) return 0; } +/* intended to be used as a SIGWINCH handler */ +void columns_reset(int signum) +{ + cached_columns = 0; +} + /* gets the current screen column width */ unsigned short getcols(int fd) { const unsigned short default_tty = 80; const unsigned short default_notty = 0; - unsigned short termwidth = 0; + unsigned short columns = 0; + + if (cached_columns > 0) + return cached_columns; if(!isatty(fd)) { return default_notty; @@ -169,15 +179,17 @@ unsigned short getcols(int fd) #if defined(TIOCGSIZE) struct ttysize win; if(ioctl(fd, TIOCGSIZE, &win) == 0) { - termwidth = win.ts_cols; + columns = win.ts_cols; } #elif defined(TIOCGWINSZ) struct winsize win; if(ioctl(fd, TIOCGWINSZ, &win) == 0) { - termwidth = win.ws_col; + columns = win.ws_col; } #endif - return termwidth == 0 ? default_tty : termwidth; + + cached_columns = columns == 0 ? default_tty : columns; + return cached_columns; } /* does the same thing as 'rm -rf' */ diff --git a/src/pacman/util.h b/src/pacman/util.h index e2297f8..d7f7b3c 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -49,6 +49,7 @@ int trans_init(alpm_transflag_t flags, int check_valid); int trans_release(void); int needs_root(void); int check_syncdbs(size_t need_repos, int check_valid); +void columns_reset(int signum); unsigned short getcols(int fd); int rmrf(const char *path); void indentprint(const char *str, unsigned short indent, unsigned short cols); -- 1.8.4.2
Instead of doing ioctl each time, cache the result. Handle SIGWINCH to we know the size of the terminal has changed (clear the cache). --- Inspired by systemd. This is to set things up for paging, as paging would require caching this value. Once the pager is up, there's no way to poll for columns. src/pacman/pacman.c | 10 ++++++++++ src/pacman/util.c | 23 ++++++++++++++++++----- src/pacman/util.h | 1 + 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index df73bcf..1643a63 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -1044,6 +1044,16 @@ int main(int argc, char *argv[]) } } + new_action.sa_handler = columns_reset; + sigemptyset(&new_action.sa_mask); + new_action.sa_flags = 0; + + sigaction(SIGWINCH, NULL, &old_action); + if(old_action.sa_handler != SIG_IGN) { + sigaction(SIGWINCH, &new_action, NULL); + } + + /* i18n init */ #if defined(ENABLE_NLS) localize(); diff --git a/src/pacman/util.c b/src/pacman/util.c index 23eea98..1dd3235 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -46,6 +46,7 @@ #include "conf.h" #include "callback.h" +static volatile unsigned cached_columns = 0; struct table_cell_t { char *label; @@ -155,29 +156,41 @@ static int flush_term_input(int fd) return 0; } +/* intended to be used as a SIGWINCH handler */ +void columns_reset(int signum) +{ + cached_columns = 0; +} + /* gets the current screen column width */ unsigned short getcols(int fd) { const unsigned short default_tty = 80; const unsigned short default_notty = 0; - unsigned short termwidth = 0; + unsigned short columns = 0; + + if (cached_columns > 0) + return cached_columns; if(!isatty(fd)) { - return default_notty; + cached_columns = default_notty; + return cached_columns; } #if defined(TIOCGSIZE) struct ttysize win; if(ioctl(fd, TIOCGSIZE, &win) == 0) { - termwidth = win.ts_cols; + columns = win.ts_cols; } #elif defined(TIOCGWINSZ) struct winsize win; if(ioctl(fd, TIOCGWINSZ, &win) == 0) { - termwidth = win.ws_col; + columns = win.ws_col; } #endif - return termwidth == 0 ? default_tty : termwidth; + + cached_columns = columns == 0 ? default_tty : columns; + return cached_columns; } /* does the same thing as 'rm -rf' */ diff --git a/src/pacman/util.h b/src/pacman/util.h index e2297f8..d7f7b3c 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -49,6 +49,7 @@ int trans_init(alpm_transflag_t flags, int check_valid); int trans_release(void); int needs_root(void); int check_syncdbs(size_t need_repos, int check_valid); +void columns_reset(int signum); unsigned short getcols(int fd); int rmrf(const char *path); void indentprint(const char *str, unsigned short indent, unsigned short cols); -- 1.8.4.2
Instead of doing ioctl each time, cache the result. Handle SIGWINCH to we know the size of the terminal has changed (clear the cache). --- Whitespace problems. God damn it, sorry for the spam, its been a while since I've done this. I'll take more care in the future. src/pacman/pacman.c | 10 ++++++++++ src/pacman/util.c | 24 +++++++++++++++++++----- src/pacman/util.h | 1 + 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index df73bcf..1643a63 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -1044,6 +1044,16 @@ int main(int argc, char *argv[]) } } + new_action.sa_handler = columns_reset; + sigemptyset(&new_action.sa_mask); + new_action.sa_flags = 0; + + sigaction(SIGWINCH, NULL, &old_action); + if(old_action.sa_handler != SIG_IGN) { + sigaction(SIGWINCH, &new_action, NULL); + } + + /* i18n init */ #if defined(ENABLE_NLS) localize(); diff --git a/src/pacman/util.c b/src/pacman/util.c index 23eea98..b08ad43 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -46,6 +46,7 @@ #include "conf.h" #include "callback.h" +static volatile unsigned cached_columns = 0; struct table_cell_t { char *label; @@ -155,29 +156,42 @@ static int flush_term_input(int fd) return 0; } +/* intended to be used as a SIGWINCH handler */ +void columns_reset(int signum) +{ + cached_columns = 0; +} + /* gets the current screen column width */ unsigned short getcols(int fd) { const unsigned short default_tty = 80; const unsigned short default_notty = 0; - unsigned short termwidth = 0; + unsigned short columns = 0; + + if(cached_columns > 0) { + return cached_columns; + } if(!isatty(fd)) { - return default_notty; + cached_columns = default_notty; + return cached_columns; } #if defined(TIOCGSIZE) struct ttysize win; if(ioctl(fd, TIOCGSIZE, &win) == 0) { - termwidth = win.ts_cols; + columns = win.ts_cols; } #elif defined(TIOCGWINSZ) struct winsize win; if(ioctl(fd, TIOCGWINSZ, &win) == 0) { - termwidth = win.ws_col; + columns = win.ws_col; } #endif - return termwidth == 0 ? default_tty : termwidth; + + cached_columns = columns == 0 ? default_tty : columns; + return cached_columns; } /* does the same thing as 'rm -rf' */ diff --git a/src/pacman/util.h b/src/pacman/util.h index e2297f8..d7f7b3c 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -49,6 +49,7 @@ int trans_init(alpm_transflag_t flags, int check_valid); int trans_release(void); int needs_root(void); int check_syncdbs(size_t need_repos, int check_valid); +void columns_reset(int signum); unsigned short getcols(int fd); int rmrf(const char *path); void indentprint(const char *str, unsigned short indent, unsigned short cols); -- 1.8.4.2
On 23/11/13 16:38, Simon Gomizelj wrote:
Instead of doing ioctl each time, cache the result. Handle SIGWINCH to we know the size of the terminal has changed (clear the cache). --- Whitespace problems.
God damn it, sorry for the spam, its been a while since I've done this. I'll take more care in the future.
Finally got around to this... Sorry for the delay. Apart from the minor comments below, I notice a lot of the variable/function names are a bit vague. As an example, this change: - unsigned short termwidth = 0; + unsigned short columns = 0; The old variable name is much more descriptive. Please prefix "columns" in all its uses with (e.g.) term so we know what columns is referring to.
src/pacman/pacman.c | 10 ++++++++++ src/pacman/util.c | 24 +++++++++++++++++++----- src/pacman/util.h | 1 + 3 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index df73bcf..1643a63 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -1044,6 +1044,16 @@ int main(int argc, char *argv[]) } }
Adding a comment here would be nice. e.g.: /* Set up signal handler to deal with terminal column width changes */
+ new_action.sa_handler = columns_reset; + sigemptyset(&new_action.sa_mask); + new_action.sa_flags = 0; + + sigaction(SIGWINCH, NULL, &old_action); + if(old_action.sa_handler != SIG_IGN) { + sigaction(SIGWINCH, &new_action, NULL); + } + + /* i18n init */ #if defined(ENABLE_NLS) localize(); diff --git a/src/pacman/util.c b/src/pacman/util.c index 23eea98..b08ad43 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -46,6 +46,7 @@ #include "conf.h" #include "callback.h"
+static volatile unsigned cached_columns = 0;
struct table_cell_t { char *label; @@ -155,29 +156,42 @@ static int flush_term_input(int fd) return 0; }
+/* intended to be used as a SIGWINCH handler */ +void columns_reset(int signum) +{ + cached_columns = 0; +} +
util.c: In function ‘columns_reset’: util.c:160:24: error: unused parameter ‘signum’ [-Werror=unused-parameter] void columns_reset(int signum) ^ We need: void columns_reset(int __attribute__((unused)) signum) or we could move the UNUSED define into src/common/util-common.h
/* gets the current screen column width */ unsigned short getcols(int fd) { const unsigned short default_tty = 80; const unsigned short default_notty = 0; - unsigned short termwidth = 0; + unsigned short columns = 0; + + if(cached_columns > 0) { + return cached_columns; + }
if(!isatty(fd)) { - return default_notty; + cached_columns = default_notty; + return cached_columns; }
#if defined(TIOCGSIZE) struct ttysize win; if(ioctl(fd, TIOCGSIZE, &win) == 0) { - termwidth = win.ts_cols; + columns = win.ts_cols; } #elif defined(TIOCGWINSZ) struct winsize win; if(ioctl(fd, TIOCGWINSZ, &win) == 0) { - termwidth = win.ws_col; + columns = win.ws_col; } #endif - return termwidth == 0 ? default_tty : termwidth; + + cached_columns = columns == 0 ? default_tty : columns; + return cached_columns; }
/* does the same thing as 'rm -rf' */ diff --git a/src/pacman/util.h b/src/pacman/util.h index e2297f8..d7f7b3c 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -49,6 +49,7 @@ int trans_init(alpm_transflag_t flags, int check_valid); int trans_release(void); int needs_root(void); int check_syncdbs(size_t need_repos, int check_valid); +void columns_reset(int signum); unsigned short getcols(int fd); int rmrf(const char *path); void indentprint(const char *str, unsigned short indent, unsigned short cols);
participants (2)
-
Allan McRae
-
Simon Gomizelj