[systemd-devel] [PATCH 3/3] journalctl: add SIGWINCH handler in --follow mode

[Lennart Poettering]

On Mon, 17.09.12 19:47, Dave Reisner (dreisner at archlinux.org) wrote:

> Recalculate the terminal size on SIGWINCH to make sure we take in to
> the account the new real estate.

Hmpf. this a bit dangerous as this code means accessing environ[] from
the signal handler, which might not safe in some cases where the main
program might be updating environ[] in some way at the point where the
signal handler is invoked.

I'd prefer if this could be reworked to simply unset the cache variable
so that the next invocation of columns() from the main context would
invoke the getenv(), not the signal handler.

(yes, this is unlikely to be a real problem in journalctl, but I'd
prefer to write correct code...)

> ---
>  src/journal/journalctl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
> index 8e52dd5..a04bb05 100644
> --- a/src/journal/journalctl.c
> +++ b/src/journal/journalctl.c
> @@ -29,6 +29,7 @@
>  #include <sys/poll.h>
>  #include <time.h>
>  #include <getopt.h>
> +#include <signal.h>
>  #include <sys/stat.h>
>  #include <sys/ioctl.h>
>  #include <linux/fs.h>
> @@ -78,6 +79,10 @@ static enum {
>          ACTION_DISK_USAGE,
>  } arg_action = ACTION_SHOW;
>  
> +static recalc_columns(int _unused_ signum) {
> +        columns_uncached();
> +}
> +
>  static int help(void) {
>  
>          printf("%s [OPTIONS...] [MATCH]\n\n"
> @@ -175,6 +180,7 @@ static int parse_argv(int argc, char *argv[]) {
>  
>                  case 'f':
>                          arg_follow = true;
> +                        signal(SIGWINCH, recalc_columns);
>                          break;
>  
>                  case 'o':


Lennart

-- 
Lennart Poettering - Red Hat, Inc.