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);