[systemd-devel] [RFC][PATCH 1/2] utmp-wtmp: allow overriding username on wall

[Zbigniew Jędrzejewski-Szmek]

Hi,
both patches look useful. Some comments below.

On Wed, Mar 05, 2014 at 01:46:01PM +0100, Sebastian Thorarensen wrote:
> utmp_wall() now takes an optional argument 'username_override' which
> allows the caller to override the username shown on wall messages.
> journald will use this to inform users that its wall messages comes from
> 'systemd-journald'.
> ---
>  src/shared/utmp-wtmp.c                              |   12 +++++++-----
>  src/shared/utmp-wtmp.h                              |    2 +-
>  src/shutdownd/shutdownd.c                           |    2 +-
>  src/systemctl/systemctl.c                           |    4 ++--
>  src/tty-ask-password-agent/tty-ask-password-agent.c |    2 +-
>  5 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/src/shared/utmp-wtmp.c b/src/shared/utmp-wtmp.c
> index 32996fa..0200798 100644
> --- a/src/shared/utmp-wtmp.c
> +++ b/src/shared/utmp-wtmp.c
> @@ -347,16 +347,18 @@ static int write_to_terminal(const char *tty, const char *message) {
>          return 0;
>  }
>  
> -int utmp_wall(const char *message, bool (*match_tty)(const char *tty)) {
> +int utmp_wall(const char *message, const char *username_override, bool (*match_tty)(const char *tty)) {
>          _cleanup_free_ char *text = NULL, *hn = NULL, *un = NULL, *tty = NULL;
>          char date[FORMAT_TIMESTAMP_MAX];
>          struct utmpx *u;
>          int r;
>  
> -        hn = gethostname_malloc();
> -        un = getlogname_malloc();
> -        if (!hn || !un)
> +        if (!(hn = gethostname_malloc()))
>                  return -ENOMEM;
> +        if (!username_override) {
> +                if (!(un = getlogname_malloc()))
> +                        return -ENOMEM;
> +        }
Please keep the assignments out of the if conditionals. And maybe
call username_override just username?

>          getttyname_harder(STDIN_FILENO, &tty);
>  
> @@ -364,7 +366,7 @@ int utmp_wall(const char *message, bool (*match_tty)(const char *tty)) {
>                       "\a\r\n"
>                       "Broadcast message from %s@%s%s%s (%s):\r\n\r\n"
>                       "%s\r\n\r\n",
> -                     un, hn,
> +                     un ? un : username_override, hn,
un ?: username_override,

>                       tty ? " on " : "", strempty(tty),
>                       format_timestamp(date, sizeof(date), now(CLOCK_REALTIME)),
>                       message) < 0)
> diff --git a/src/shared/utmp-wtmp.h b/src/shared/utmp-wtmp.h
> index 5924023..f15bbf7 100644
> --- a/src/shared/utmp-wtmp.h
> +++ b/src/shared/utmp-wtmp.h
> @@ -32,4 +32,4 @@ int utmp_put_runlevel(int runlevel, int previous);
>  int utmp_put_dead_process(const char *id, pid_t pid, int code, int status);
>  int utmp_put_init_process(const char *id, pid_t pid, pid_t sid, const char *line);
>  
> -int utmp_wall(const char *message, bool (*match_tty)(const char *tty));
> +int utmp_wall(const char *message, const char *username_override, bool (*match_tty)(const char *tty));
> diff --git a/src/shutdownd/shutdownd.c b/src/shutdownd/shutdownd.c
> index 8d034e6..fafd9ce 100644
> --- a/src/shutdownd/shutdownd.c
> +++ b/src/shutdownd/shutdownd.c
> @@ -143,7 +143,7 @@ static void warn_wall(usec_t n, struct sd_shutdown_command *c) {
>  
>          if (asprintf(&l, "%s%s%s%s!", c->wall_message, c->wall_message[0] ? "\n" : "",
>                       prefix, format_timestamp(date, sizeof(date), c->usec)) >= 0)
> -                utmp_wall(l, NULL);
> +                utmp_wall(l, NULL, NULL);
>          else
>                  log_error("Failed to allocate wall message");
>  }
> diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
> index f395265..0435d40 100644
> --- a/src/systemctl/systemctl.c
> +++ b/src/systemctl/systemctl.c
> @@ -257,7 +257,7 @@ static void warn_wall(enum action a) {
>                  }
>  
>                  if (*p) {
> -                        utmp_wall(p, NULL);
> +                        utmp_wall(p, NULL, NULL);
>                          return;
>                  }
>          }
> @@ -265,7 +265,7 @@ static void warn_wall(enum action a) {
>          if (!table[a])
>                  return;
>  
> -        utmp_wall(table[a], NULL);
> +        utmp_wall(table[a], NULL, NULL);
>  }
>  
>  static bool avoid_bus(void) {
> diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c
> index 7a90e65..fa4d660 100644
> --- a/src/tty-ask-password-agent/tty-ask-password-agent.c
> +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
> @@ -538,7 +538,7 @@ static int show_passwords(void) {
>                  free(p);
>  
>                  if (wall) {
> -                        utmp_wall(wall, wall_tty_match);
> +                        utmp_wall(wall, NULL, wall_tty_match);
>                          free(wall);
>                  }
>          }

Zbyszek