Merge branch 'write-improvements' of git://github.com/kerolasa/lelux-utiliteetit

* 'write-improvements' of git://github.com/kerolasa/lelux-utiliteetit:
  lib: try to find tty in get_terminal_name()
  write: stop removing and adding /dev/ in front of tty string
  write: tell when effective gid and tty path group mismatch
  write: improve coding style
  write: remove PUTC macro
  write: make timestamp to be obviously just a clock time
  write: remove unnecessary utmp variables
  write: improve function and variable names
  write: add control structure to clarify what is going on
  write: run atexit() checks at the end of execution
  write: use xstrncpy() from strutils.h
  write: set atime value in term_chk() only when needed
  write: remove pointless fileno(3) calls
  write: get rid of function prototypes
  write: remove unused variable

6 files changed, 216 insertions, 224 deletions

diff --git a/include/ttyutils.h b/include/ttyutils.h
index 200e9a565..7278d3615 100644
--- a/include/ttyutils.h
+++ b/include/ttyutils.h
@@ -51,7 +51,7 @@ struct chardata {
 	} while (0)
 
 extern int get_terminal_width(int default_width);
-extern int get_terminal_name(int fd, const char **path, const char **name,
+extern int get_terminal_name(const char **path, const char **name,
 			     const char **number);
 
 #define UL_TTY_KEEPCFLAGS	(1 << 1)
diff --git a/lib/ttyutils.c b/lib/ttyutils.c
index 4e62c20d6..06dd800f7 100644
--- a/lib/ttyutils.c
+++ b/lib/ttyutils.c
@@ -5,6 +5,7 @@
  * Written by Karel Zak <kzak@redhat.com>
  */
 #include <ctype.h>
+#include <unistd.h>
 
 #include "c.h"
 #include "ttyutils.h"
@@ -42,13 +43,14 @@ int get_terminal_width(int default_width)
 	return width > 0 ? width : default_width;
 }
 
-int get_terminal_name(int fd,
-		      const char **path,
+int get_terminal_name(const char **path,
 		      const char **name,
 		      const char **number)
 {
 	const char *tty;
 	const char *p;
+	int fd;
+
 
 	if (name)
 		*name = NULL;
@@ -57,6 +59,15 @@ int get_terminal_name(int fd,
 	if (number)
 		*number = NULL;
 
+	if (isatty(STDIN_FILENO))
+		fd = STDIN_FILENO;
+	else if (isatty(STDOUT_FILENO))
+		fd = STDOUT_FILENO;
+	else if (isatty(STDERR_FILENO))
+		fd = STDERR_FILENO;
+	else
+		return -1;
+
 	tty = ttyname(fd);
 	if (!tty)
 		return -1;
diff --git a/login-utils/login.c b/login-utils/login.c
index 4d71f5424..7501f6d69 100644
--- a/login-utils/login.c
+++ b/login-utils/login.c
@@ -356,7 +356,7 @@ static void init_tty(struct login_context *cxt)
 
 	cxt->tty_mode = (mode_t) getlogindefs_num("TTYPERM", TTY_MODE);
 
-	get_terminal_name(0, &cxt->tty_path, &cxt->tty_name, &cxt->tty_number);
+	get_terminal_name(&cxt->tty_path, &cxt->tty_name, &cxt->tty_number);
 
 	/*
 	 * In case login is suid it was possible to use a hardlink as stdin
diff --git a/login-utils/su-common.c b/login-utils/su-common.c
index 78a734c67..1776b6b79 100644
--- a/login-utils/su-common.c
+++ b/login-utils/su-common.c
@@ -165,7 +165,7 @@ log_syslog(struct passwd const *pw, bool successful)
       old_user = pwd ? pwd->pw_name : "";
     }
 
-  if (get_terminal_name(STDERR_FILENO, NULL, &tty, NULL) != 0 || !tty)
+  if (get_terminal_name(NULL, &tty, NULL) != 0 || !tty)
     tty = "none";
 
   openlog (program_invocation_short_name, 0 , LOG_AUTH);
@@ -192,7 +192,7 @@ static void log_btmp(struct passwd const *pw)
 		pw && pw->pw_name ? pw->pw_name : "(unknown)",
 		sizeof(ut.ut_user));
 
-	get_terminal_name(STDERR_FILENO, NULL, &tty_name, &tty_num);
+	get_terminal_name(NULL, &tty_name, &tty_num);
 	if (tty_num)
 		xstrncpy(ut.ut_id, tty_num, sizeof(ut.ut_id));
 	if (tty_name)
diff --git a/term-utils/Makemodule.am b/term-utils/Makemodule.am
index 8ddb0687b..1b7c5fccf 100644
--- a/term-utils/Makemodule.am
+++ b/term-utils/Makemodule.am
@@ -93,6 +93,7 @@ dist_man_MANS += term-utils/write.1
 write_SOURCES = term-utils/write.c
 write_CFLAGS = $(SUID_CFLAGS) $(AM_CFLAGS)
 write_LDFLAGS = $(SUID_LDFLAGS) $(AM_LDFLAGS)
+write_LDADD = $(LDADD) libcommon.la
 
 if USE_TTY_GROUP
 if MAKEINSTALL_DO_CHOWN
diff --git a/term-utils/write.c b/term-utils/write.c
index 7c1523136..6bd393441 100644
--- a/term-utils/write.c
+++ b/term-utils/write.c
@@ -45,37 +45,41 @@
  *
  */
 
-#include <stdio.h>
-#include <unistd.h>
-#include <utmp.h>
 #include <errno.h>
-#include <time.h>
+#include <getopt.h>
+#include <paths.h>
 #include <pwd.h>
-#include <string.h>
-#include <stdlib.h>
 #include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
 #include <sys/param.h>
 #include <sys/stat.h>
-#include <paths.h>
-#include <getopt.h>
+#include <time.h>
+#include <unistd.h>
+#include <utmp.h>
 
 #include "c.h"
 #include "carefulputc.h"
 #include "closestream.h"
 #include "nls.h"
+#include "strutils.h"
+#include "ttyutils.h"
 #include "xalloc.h"
 
-static void __attribute__ ((__noreturn__)) usage(FILE * out);
-void search_utmp(char *, char *, char *, uid_t);
-void do_write(char *, char *, uid_t);
-void wr_fputs(char *);
-static void __attribute__ ((__noreturn__)) done(int);
-int term_chk(char *, int *, time_t *, int);
-int utmp_chk(char *, char *);
+static sig_atomic_t signal_received = 0;
 
-static gid_t root_access;
+struct write_control {
+	uid_t src_uid;
+	const char *src_login;
+	const char *src_tty_path;
+	const char *src_tty_name;
+	const char *dst_login;
+	char *dst_tty_path;
+	const char *dst_tty_name;
+};
 
-static void __attribute__ ((__noreturn__)) usage(FILE * out)
+static void __attribute__((__noreturn__)) usage(FILE *out)
 {
 	fputs(USAGE_HEADER, out);
 	fprintf(out,
@@ -93,118 +97,48 @@ static void __attribute__ ((__noreturn__)) usage(FILE * out)
 	exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
 }
 
-int main(int argc, char **argv)
+/*
+ * check_tty - check that a terminal exists, and get the message bit
+ *     and the access time
+ */
+static int check_tty(const char *tty, int *tty_writeable, time_t *tty_atime, int showerror)
 {
-	time_t atime;
-	uid_t myuid;
-	int msgsok = 0, myttyfd, c;
-	char tty[PATH_MAX], *mytty;
-
-	static const struct option longopts[] = {
-		{"version", no_argument, NULL, 'V'},
-		{"help", no_argument, NULL, 'h'},
-		{NULL, 0, NULL, 0}
-	};
-
-	setlocale(LC_ALL, "");
-	bindtextdomain(PACKAGE, LOCALEDIR);
-	textdomain(PACKAGE);
-	atexit(close_stdout);
+	struct stat s;
 
-	while ((c = getopt_long(argc, argv, "Vh", longopts, NULL)) != -1)
-		switch (c) {
-		case 'V':
-			printf(UTIL_LINUX_VERSION);
-			return EXIT_SUCCESS;
-		case 'h':
-			usage(stdout);
-		default:
-			usage(stderr);
+	if (stat(tty, &s) < 0) {
+		if (showerror)
+			warn("%s", tty);
+		return 1;
+	}
+	if (getuid() == 0)	/* root can always write */
+		*tty_writeable = 1;
+	else {
+		if (getegid() != s.st_gid) {
+			warnx(_("effective gid does not match group of %s"), tty);
+			return 1;
 		}
-
-	root_access = !getegid();
-
-	/* check that sender has write enabled */
-	if (isatty(fileno(stdin)))
-		myttyfd = fileno(stdin);
-	else if (isatty(fileno(stdout)))
-		myttyfd = fileno(stdout);
-	else if (isatty(fileno(stderr)))
-		myttyfd = fileno(stderr);
-	else
-		myttyfd = -1;
-
-	if (myttyfd != -1) {
-		if (!(mytty = ttyname(myttyfd)))
-			errx(EXIT_FAILURE,
-			     _("can't find your tty's name"));
-
-		/*
-		 * We may have /dev/ttyN but also /dev/pts/xx. Below,
-		 * term_chk() will put "/dev/" in front, so remove that
-		 * part.
-		 */
-		if (!strncmp(mytty, "/dev/", 5))
-			mytty += 5;
-		if (term_chk(mytty, &msgsok, &atime, 1))
-			exit(EXIT_FAILURE);
-		if (!msgsok)
-			errx(EXIT_FAILURE,
-			     _("you have write permission turned off"));
-		msgsok = 0;
-	} else
-		mytty = "<no tty>";
-
-	myuid = getuid();
-
-	/* check args */
-	switch (argc) {
-	case 2:
-		search_utmp(argv[1], tty, mytty, myuid);
-		do_write(tty, mytty, myuid);
-		break;
-	case 3:
-		if (!strncmp(argv[2], "/dev/", 5))
-			argv[2] += 5;
-		if (utmp_chk(argv[1], argv[2]))
-			errx(EXIT_FAILURE,
-			     _("%s is not logged in on %s"),
-			     argv[1], argv[2]);
-		if (term_chk(argv[2], &msgsok, &atime, 1))
-			exit(EXIT_FAILURE);
-		if (myuid && !msgsok)
-			errx(EXIT_FAILURE,
-			     _("%s has messages disabled on %s"),
-			     argv[1], argv[2]);
-		do_write(argv[2], mytty, myuid);
-		break;
-	default:
-		usage(stderr);
+		*tty_writeable = s.st_mode & S_IWGRP;
 	}
-
-	done(0);
-	/* NOTREACHED */
-	return EXIT_FAILURE;
+	if (tty_atime)
+		*tty_atime = s.st_atime;
+	return 0;
 }
 
-
 /*
- * utmp_chk - checks that the given user is actually logged in on
+ * check_utmp - checks that the given user is actually logged in on
  *     the given tty
  */
-int utmp_chk(char *user, char *tty)
+static int check_utmp(const struct write_control *ctl)
 {
-	struct utmp u;
-	struct utmp *uptr;
+	struct utmp *u;
 	int res = 1;
 
 	utmpname(_PATH_UTMP);
 	setutent();
 
-	while ((uptr = getutent())) {
-		memcpy(&u, uptr, sizeof(u));
-		if (strncmp(user, u.ut_user, sizeof(u.ut_user)) == 0 &&
-		    strncmp(tty, u.ut_line, sizeof(u.ut_line)) == 0) {
+	while ((u = getutent())) {
+		if (strncmp(ctl->dst_login, u->ut_user, sizeof(u->ut_user)) == 0 &&
+		    strncmp(ctl->dst_tty_name, u->ut_line, sizeof(u->ut_line)) == 0) {
 			res = 0;
 			break;
 		}
@@ -225,164 +159,210 @@ int utmp_chk(char *user, char *tty)
  * Special case for writing to yourself - ignore the terminal you're
  * writing from, unless that's the only terminal with messages enabled.
  */
-void search_utmp(char *user, char *tty, char *mytty, uid_t myuid)
+static void search_utmp(struct write_control *ctl)
 {
-	struct utmp u;
-	struct utmp *uptr;
-	time_t bestatime, atime;
-	int nloggedttys, nttys, msgsok = 0, user_is_me;
-	char atty[sizeof(u.ut_line) + 1];
+	struct utmp *u;
+	time_t best_atime = 0, tty_atime;
+	int num_ttys = 0, valid_ttys = 0, tty_writeable = 0, user_is_me = 0;
+	char path[UT_LINESIZE + 6];
 
 	utmpname(_PATH_UTMP);
 	setutent();
 
-	nloggedttys = nttys = 0;
-	bestatime = 0;
-	user_is_me = 0;
-	while ((uptr = getutent())) {
-		memcpy(&u, uptr, sizeof(u));
-		if (strncmp(user, u.ut_user, sizeof(u.ut_user)) == 0) {
-			++nloggedttys;
-			strncpy(atty, u.ut_line, sizeof(u.ut_line));
-			atty[sizeof(u.ut_line)] = '\0';
-			if (term_chk(atty, &msgsok, &atime, 0))
-				/* bad term? skip */
-				continue;
-			if (myuid && !msgsok)
-				/* skip ttys with msgs off */
-				continue;
-			if (strcmp(atty, mytty) == 0) {
-				user_is_me = 1;
-				/* don't write to yourself */
-				continue;
-			}
-			if (u.ut_type != USER_PROCESS)
-				/* it's not a valid entry */
-				continue;
-			++nttys;
-			if (atime > bestatime) {
-				bestatime = atime;
-				strcpy(tty, atty);
-			}
+	while ((u = getutent())) {
+		if (strncmp(ctl->dst_login, u->ut_user, sizeof(u->ut_user)) != 0)
+			continue;
+		num_ttys++;
+		sprintf(path, "/dev/%s", u->ut_line);
+		if (check_tty(path, &tty_writeable, &tty_atime, 0))
+			/* bad term? skip */
+			continue;
+		if (ctl->src_uid && !tty_writeable)
+			/* skip ttys with msgs off */
+			continue;
+		if (strcmp(u->ut_line, ctl->src_tty_name) == 0) {
+			user_is_me = 1;
+			/* don't write to yourself */
+			continue;
+		}
+		if (u->ut_type != USER_PROCESS)
+			/* it's not a valid entry */
+			continue;
+		valid_ttys++;
+		if (best_atime < tty_atime) {
+			best_atime = tty_atime;
+			free(ctl->dst_tty_path);
+			ctl->dst_tty_path = xstrdup(path);
+			ctl->dst_tty_name = ctl->dst_tty_path + 5;
 		}
 	}
 
 	endutent();
-	if (nloggedttys == 0)
-		errx(EXIT_FAILURE, _("%s is not logged in"), user);
-	if (nttys == 0) {
+	if (num_ttys == 0)
+		errx(EXIT_FAILURE, _("%s is not logged in"), ctl->dst_login);
+	if (valid_ttys == 0) {
 		if (user_is_me) {
 			/* ok, so write to yourself! */
-			strcpy(tty, mytty);
+			if (!ctl->src_tty_path)
+				errx(EXIT_FAILURE, _("can't find your tty's name"));
+			ctl->dst_tty_path = xstrdup(ctl->src_tty_path);
+			ctl->dst_tty_name = ctl->dst_tty_path + 5;
 			return;
 		}
-		errx(EXIT_FAILURE, _("%s has messages disabled"), user);
-	} else if (nttys > 1) {
-		warnx(_("%s is logged in more than once; writing to %s"),
-		      user, tty);
+		errx(EXIT_FAILURE, _("%s has messages disabled"), ctl->dst_login);
 	}
+	if (1 < valid_ttys)
+		warnx(_("%s is logged in more than once; writing to %s"),
+		      ctl->dst_login, ctl->dst_tty_name);
 }
 
 /*
- * term_chk - check that a terminal exists, and get the message bit
- *     and the access time
+ * signal_handler - cause write loop to exit
  */
-int term_chk(char *tty, int *msgsokP, time_t * atimeP, int showerror)
+static void signal_handler(int signo)
 {
-	struct stat s;
-	char path[PATH_MAX];
+	signal_received = signo;
+}
 
-	if (strlen(tty) + 6 > sizeof(path))
-		return 1;
-	sprintf(path, "/dev/%s", tty);
-	if (stat(path, &s) < 0) {
-		if (showerror)
-			warn("%s", path);
-		return 1;
+/*
+ * write_line - like fputs(), but makes control characters visible and
+ *     turns \n into \r\n.
+ */
+static void write_line(char *s)
+{
+	while (*s) {
+		const int c = *s++;
+
+		if ((c == '\n' && fputc_careful('\r', stdout, '^') == EOF)
+		    || fputc_careful(c, stdout, '^') == EOF)
+			err(EXIT_FAILURE, _("carefulputc failed"));
 	}
-	if (getuid() == 0)	/* root can always write */
-		*msgsokP = 1;
-	else
-		*msgsokP = (s.st_mode & S_IWGRP) && (getegid() == s.st_gid);
-	*atimeP = s.st_atime;
-	return 0;
 }
 
 /*
  * do_write - actually make the connection
  */
-void do_write(char *tty, char *mytty, uid_t myuid)
+static void do_write(const struct write_control *ctl)
 {
-	char *login, *pwuid, *nows;
+	char *login, *pwuid, timestamp[6];
 	struct passwd *pwd;
 	time_t now;
-	char path[PATH_MAX], *host, line[512];
+	struct tm *tm;
+	char *host, line[512];
+	struct sigaction sigact;
 
 	/* Determine our login name(s) before the we reopen() stdout */
-	if ((pwd = getpwuid(myuid)) != NULL)
+	if ((pwd = getpwuid(ctl->src_uid)) != NULL)
 		pwuid = pwd->pw_name;
 	else
 		pwuid = "???";
 	if ((login = getlogin()) == NULL)
 		login = pwuid;
 
-	if (strlen(tty) + 6 > sizeof(path))
-		errx(EXIT_FAILURE, _("tty path %s too long"), tty);
-	snprintf(path, sizeof(path), "/dev/%s", tty);
-	if ((freopen(path, "w", stdout)) == NULL)
-		err(EXIT_FAILURE, "%s", path);
+	if ((freopen(ctl->dst_tty_path, "w", stdout)) == NULL)
+		err(EXIT_FAILURE, "%s", ctl->dst_tty_path);
 
-	signal(SIGINT, done);
-	signal(SIGHUP, done);
+	sigact.sa_handler = signal_handler;
+	sigemptyset(&sigact.sa_mask);
+	sigact.sa_flags = 0;
+	sigaction(SIGINT, &sigact, NULL);
+	sigaction(SIGHUP, &sigact, NULL);
 
-	/* print greeting */
 	host = xgethostname();
 	if (!host)
 		host = xstrdup("???");
 
-	now = time((time_t *) NULL);
-	nows = ctime(&now);
-	nows[16] = '\0';
-	printf("\r\n\007\007\007");
+	now = time((time_t *)NULL);
+	tm = localtime(&now);
+	strftime(timestamp, sizeof(timestamp), "%H:%M", tm);
+	/* print greeting */
+	printf("\r\n\a\a\a");
 	if (strcmp(login, pwuid))
 		printf(_("Message from %s@%s (as %s) on %s at %s ..."),
-		       login, host, pwuid, mytty, nows + 11);
+		       login, host, pwuid, ctl->src_tty_name, timestamp);
 	else
 		printf(_("Message from %s@%s on %s at %s ..."),
-		       login, host, mytty, nows + 11);
+		       login, host, ctl->src_tty_name, timestamp);
 	free(host);
 	printf("\r\n");
 
-	while (fgets(line, sizeof(line), stdin) != NULL)
-		wr_fputs(line);
-}
-
-/*
- * done - cleanup and exit
- */
-static void __attribute__ ((__noreturn__))
-    done(int dummy __attribute__ ((__unused__)))
-{
+	while (fgets(line, sizeof(line), stdin) != NULL) {
+		if (signal_received)
+			break;
+		write_line(line);
+	}
 	printf("EOF\r\n");
-	_exit(EXIT_SUCCESS);
 }
 
-/*
- * wr_fputs - like fputs(), but makes control characters visible and
- *     turns \n into \r\n.
- */
-void wr_fputs(char *s)
+int main(int argc, char **argv)
 {
-	char c;
+	int tty_writeable = 0, c;
+	struct write_control ctl = { 0 };
 
-#define	PUTC(c)	if (fputc_careful(c, stdout, '^') == EOF) \
-    err(EXIT_FAILURE, _("carefulputc failed"));
-	while (*s) {
-		c = *s++;
-		if (c == '\n')
-			PUTC('\r');
-		PUTC(c);
+	static const struct option longopts[] = {
+		{"version", no_argument, NULL, 'V'},
+		{"help", no_argument, NULL, 'h'},
+		{NULL, 0, NULL, 0}
+	};
+
+	setlocale(LC_ALL, "");
+	bindtextdomain(PACKAGE, LOCALEDIR);
+	textdomain(PACKAGE);
+	atexit(close_stdout);
+
+	while ((c = getopt_long(argc, argv, "Vh", longopts, NULL)) != -1)
+		switch (c) {
+		case 'V':
+			printf(UTIL_LINUX_VERSION);
+			return EXIT_SUCCESS;
+		case 'h':
+			usage(stdout);
+		default:
+			usage(stderr);
+		}
+
+	if (get_terminal_name(&ctl.src_tty_path, &ctl.src_tty_name, NULL) == 0) {
+		/* check that sender has write enabled */
+		if (check_tty(ctl.src_tty_path, &tty_writeable, NULL, 1))
+			exit(EXIT_FAILURE);
+		if (!tty_writeable)
+			errx(EXIT_FAILURE,
+			     _("you have write permission turned off"));
+		tty_writeable = 0;
+	} else
+		ctl.src_tty_name = "<no tty>";
+
+	ctl.src_uid = getuid();
+
+	/* check args */
+	switch (argc) {
+	case 2:
+		ctl.dst_login = argv[1];
+		search_utmp(&ctl);
+		do_write(&ctl);
+		break;
+	case 3:
+		ctl.dst_login = argv[1];
+		if (!strncmp(argv[2], "/dev/", 5))
+			ctl.dst_tty_path = xstrdup(argv[2]);
+		else
+			xasprintf(&ctl.dst_tty_path, "/dev/%s", argv[2]);
+		ctl.dst_tty_name = ctl.dst_tty_path + 5;
+		if (check_utmp(&ctl))
+			errx(EXIT_FAILURE,
+			     _("%s is not logged in on %s"),
+			     ctl.dst_login, ctl.dst_tty_name);
+		if (check_tty(ctl.dst_tty_path, &tty_writeable, NULL, 1))
+			exit(EXIT_FAILURE);
+		if (ctl.src_uid && !tty_writeable)
+			errx(EXIT_FAILURE,
+			     _("%s has messages disabled on %s"),
+			     ctl.dst_login, ctl.dst_tty_name);
+		do_write(&ctl);
+		break;
+	default:
+		usage(stderr);
 	}
-	return;
-#undef PUTC
+	free(ctl.dst_tty_path);
+	return EXIT_SUCCESS;
 }