[pacman-dev] [PATCH 2/2] cache terminal size, add SIGWINCH handler to reset

[Andrew Gregory]

On 06/24/14 at 04:32pm, Dave Reisner wrote:
> Refactoring getcols, yet again. We do the following:
> 
> 1) Introduce a static global in src/pacman/util.c
> 2) getcols always prefers this cached value, but will derive it from
> the COLUMNS environment var, the characteristics of stdout, or a sane
> default (in that order).
> 3) Introduce a SIGWINCH signal handler to reset the cached value,
> meaning we only call ioctl when we don't know the value.
> 
> On my machine, pacman -Syy goes from ~4300 ioctl calls to 3.
> ---
>  src/pacman/callback.c |  6 +++---
>  src/pacman/package.c  |  4 ++--
>  src/pacman/pacman.c   |  5 ++++-
>  src/pacman/util.c     | 59 +++++++++++++++++++++++++++++++++++++--------------
>  src/pacman/util.h     |  3 ++-
>  5 files changed, 54 insertions(+), 23 deletions(-)
 
A few small nitpicks.

<snip>

> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index ebf15db..9244870 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -46,6 +46,7 @@
>  #include "conf.h"
>  #include "callback.h"
>  
> +static int cached_columns = 0;
>  
>  struct table_cell_t {
>  	char *label;
> @@ -155,29 +156,55 @@ static int flush_term_input(int fd)
>  	return 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;
> +void columns_cache_reset() {

Opening brace needs to be on its own line and missing void parameter.

> +	cached_columns = 0;
> +}
>  
> -	if(!isatty(fd)) {
> -		return default_notty;
> -	}
> +static int getcols_fd(int fd) {

Opening brace.

> +	int width = -1;
>  
>  #if defined(TIOCGSIZE)
>  	struct ttysize win;
>  	if(ioctl(fd, TIOCGSIZE, &win) == 0) {
> -		termwidth = win.ts_cols;
> +		width = win.ts_cols;
>  	}
>  #elif defined(TIOCGWINSZ)
>  	struct winsize win;
>  	if(ioctl(fd, TIOCGWINSZ, &win) == 0) {
> -		termwidth = win.ws_col;
> +		width = win.ws_col;
>  	}
>  #endif
> -	return termwidth == 0 ? default_tty : termwidth;
> +
> +	if(width <= 0) {
> +		return -EIO;
> +	}
> +
> +	return width;
> +}
> +
> +unsigned short getcols(void) {

Opening brace.

> +	const char *e;
> +	int c = 0;
> +
> +	if(cached_columns > 0) {
> +		return cached_columns;
> +	}
> +
> +	e = getenv("COLUMNS");
> +	if(e) {
> +		c = strtol(e, NULL, 10);
> +	}
> +
> +	if(c <= 0) {
> +		c = getcols_fd(STDOUT_FILENO);
> +	}
> +
> +	if(c <= 0) {
> +		c = 80;
> +	}
> +
> +	cached_columns = c;
> +	return c;
>  }
>  
>  /* does the same thing as 'rm -rf' */