[PATCH 00/14] pull: chsh, newgrp, and script changes

[PATCH 00/14] pull: chsh, newgrp, and script changes

[Sami Kerola]

Hi Karel,

Here are the last change I want to sent before holiday break.

This patch set is primarily about chsh(1).  Changes to newgrp(1) are
modest, and the changes to script(1) are incomplete.  Consider the
script(1) changes as preparation to fixes ahead.  I ACK future
exepectations for script are described in the test file.

$ grep -A1 TODO tests/ts/script/race 
# TODO see comments about script design
# https://github.com/karelzak/util-linux/pull/62

In between eating too much, queen's speech, pulling some crackers, and
other festive activities I might have time to further improve script(1).

Finally, here is an alternative for people who prefer using changes from
remote repository rather then merging from mails.

----------------------------------------------------------------
The following changes since commit bf6c15ed4a3e00ae1f9c18c6d9bbf8589e09a2da:
  chfn: fix compilation without libuser (2014-12-19 15:05:04 +0100)
are available in the git repository at:
  git://github.com/kerolasa/lelux-utiliteetit.git script
for you to fetch changes up to 0b3b58ae5ff95fd9cdea3e2fa45ef02639e8521d:
  script: add struct script_control and remove global variables (2014-12-20 16:03:29 +0000)
----------------------------------------------------------------

Sami Kerola (14):
  chsh: remove function prototypes
  chfn, chsh: share illegal_passwd_chars() function
  chsh: use getline() to support arbitrarily long lines
  chsh: set few variables read-only and rename one of them
  chsh: allow user to set shell to /bin/sh if none is set
  chsh: clean up parse_argv()
  chsh: rewrite function interacting with user to get path to new shell
  chsh: simplify check_shell()
  chsh: fail get_shell_list() check when /etc/shells cannot be opened
  newgrp: simplify if else clauses
  newgrp: move shell determination closer where it is used
  newgrp: set function arguments read-only when possible
  script: remove function prototypes
  script: add struct script_control and remove global variables

 login-utils/Makemodule.am |   4 +-
 login-utils/ch-common.c   |  34 ++
 login-utils/ch-common.h   |   6 +
 login-utils/chfn.c        |  16 +-
 login-utils/chsh.c        | 365 ++++++++++-----------
 login-utils/newgrp.c      |  25 +-
 term-utils/script.c       | 788 +++++++++++++++++++++++-----------------------
 7 files changed, 609 insertions(+), 629 deletions(-)
 create mode 100644 login-utils/ch-common.c
 create mode 100644 login-utils/ch-common.h

-- 
2.2.1

[PATCH 01/14] chsh: remove function prototypes

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/chsh.c | 290 ++++++++++++++++++++++++++---------------------------
 1 file changed, 143 insertions(+), 147 deletions(-)

diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index 76c3d21..1327614 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -61,10 +61,6 @@ struct sinfo {
 	char *shell;
 };
 
-static void parse_argv(int argc, char **argv, struct sinfo *pinfo);
-static char *prompt(char *question, char *def_val);
-static int check_shell(char *shell);
-static int get_shell_list(char *shell);
 
 static void __attribute__((__noreturn__)) usage (FILE *fp)
 {
@@ -80,118 +76,47 @@ static void __attribute__((__noreturn__)) usage (FILE *fp)
 	exit(fp == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
 }
 
-int main(int argc, char **argv)
+/*
+ *  get_shell_list () -- if the given shell appears in /etc/shells,
+ *	return true.  if not, return false.
+ *	if the given shell is NULL, /etc/shells is outputted to stdout.
+ */
+static int get_shell_list(char *shell_name)
 {
-	char *shell, *oldshell;
-	uid_t uid;
-	struct sinfo info;
-	struct passwd *pw;
-
-	sanitize_env();
-	setlocale(LC_ALL, "");
-	bindtextdomain(PACKAGE, LOCALEDIR);
-	textdomain(PACKAGE);
-	atexit(close_stdout);
-
-	uid = getuid();
-	memset(&info, 0, sizeof(info));
+	FILE *fp;
+	int found;
+	int len;
+	char buf[PATH_MAX];
 
-	parse_argv(argc, argv, &info);
-	pw = NULL;
-	if (!info.username) {
-		pw = getpwuid(uid);
-		if (!pw)
-			errx(EXIT_FAILURE, _("you (user %d) don't exist."),
-			     uid);
-	} else {
-		pw = getpwnam(info.username);
-		if (!pw)
-			errx(EXIT_FAILURE, _("user \"%s\" does not exist."),
-			     info.username);
+	found = false;
+	fp = fopen(_PATH_SHELLS, "r");
+	if (!fp) {
+		if (!shell_name)
+			warnx(_("No known shells."));
+		return true;
 	}
-
-#ifndef HAVE_LIBUSER
-	if (!(is_local(pw->pw_name)))
-		errx(EXIT_FAILURE, _("can only change local entries"));
-#endif
-
-#ifdef HAVE_LIBSELINUX
-	if (is_selinux_enabled() > 0) {
-		if (uid == 0) {
-			if (checkAccess(pw->pw_name, PASSWD__CHSH) != 0) {
-				security_context_t user_context;
-				if (getprevcon(&user_context) < 0)
-					user_context =
-					    (security_context_t) NULL;
-
-				errx(EXIT_FAILURE,
-				     _("%s is not authorized to change the shell of %s"),
-				     user_context ? : _("Unknown user context"),
-				     pw->pw_name);
+	while (fgets(buf, sizeof(buf), fp) != NULL) {
+		/* ignore comments */
+		if (*buf == '#')
+			continue;
+		len = strlen(buf);
+		/* strip the ending newline */
+		if (buf[len - 1] == '\n')
+			buf[len - 1] = 0;
+		/* ignore lines that are too damn long */
+		else
+			continue;
+		/* check or output the shell */
+		if (shell_name) {
+			if (!strcmp(shell_name, buf)) {
+				found = true;
+				break;
 			}
-		}
-		if (setupDefaultContext(_PATH_PASSWD) != 0)
-			errx(EXIT_FAILURE,
-			     _("can't set default context for %s"), _PATH_PASSWD);
-	}
-#endif
-
-	oldshell = pw->pw_shell;
-	if (oldshell == NULL || *oldshell == '\0')
-		oldshell = _PATH_BSHELL;	/* default */
-
-	/* reality check */
-#ifdef HAVE_LIBUSER
-	/* If we're setuid and not really root, disallow the password change. */
-	if (geteuid() != getuid() && uid != pw->pw_uid) {
-#else
-	if (uid != 0 && uid != pw->pw_uid) {
-#endif
-		errno = EACCES;
-		err(EXIT_FAILURE,
-		    _("running UID doesn't match UID of user we're "
-		      "altering, shell change denied"));
-	}
-	if (uid != 0 && !get_shell_list(oldshell)) {
-		errno = EACCES;
-		err(EXIT_FAILURE, _("your shell is not in %s, "
-				    "shell change denied"), _PATH_SHELLS);
-	}
-
-	shell = info.shell;
-
-	printf(_("Changing shell for %s.\n"), pw->pw_name);
-
-#if !defined(HAVE_LIBUSER) && defined(CHFN_CHSH_PASSWORD)
-	if(!auth_pam("chsh", uid, pw->pw_name)) {
-		return EXIT_FAILURE;
-	}
-#endif
-	if (!shell) {
-		shell = prompt(_("New shell"), oldshell);
-		if (!shell)
-			return EXIT_SUCCESS;
+		} else
+			printf("%s\n", buf);
 	}
-
-	if (check_shell(shell) < 0)
-		return EXIT_FAILURE;
-
-	if (strcmp(oldshell, shell) == 0)
-		errx(EXIT_SUCCESS, _("Shell not changed."));
-
-#ifdef HAVE_LIBUSER
-	if (set_value_libuser("chsh", pw->pw_name, uid,
-	    LU_LOGINSHELL, shell) < 0)
-		errx(EXIT_FAILURE, _("Shell *NOT* changed.  Try again later."));
-#else
-	pw->pw_shell = shell;
-	if (setpwnam(pw) < 0)
-		err(EXIT_FAILURE, _("setpwnam failed\n"
-			"Shell *NOT* changed.  Try again later."));
-#endif
-
-	printf(_("Shell changed.\n"));
-	return EXIT_SUCCESS;
+	fclose(fp);
+	return found;
 }
 
 /*
@@ -331,45 +256,116 @@ static int check_shell(char *shell)
 	return 0;
 }
 
-/*
- *  get_shell_list () -- if the given shell appears in /etc/shells,
- *	return true.  if not, return false.
- *	if the given shell is NULL, /etc/shells is outputted to stdout.
- */
-static int get_shell_list(char *shell_name)
+int main(int argc, char **argv)
 {
-	FILE *fp;
-	int found;
-	int len;
-	char buf[PATH_MAX];
+	char *shell, *oldshell;
+	uid_t uid;
+	struct sinfo info;
+	struct passwd *pw;
 
-	found = false;
-	fp = fopen(_PATH_SHELLS, "r");
-	if (!fp) {
-		if (!shell_name)
-			warnx(_("No known shells."));
-		return true;
+	sanitize_env();
+	setlocale(LC_ALL, "");
+	bindtextdomain(PACKAGE, LOCALEDIR);
+	textdomain(PACKAGE);
+	atexit(close_stdout);
+
+	uid = getuid();
+	memset(&info, 0, sizeof(info));
+
+	parse_argv(argc, argv, &info);
+	pw = NULL;
+	if (!info.username) {
+		pw = getpwuid(uid);
+		if (!pw)
+			errx(EXIT_FAILURE, _("you (user %d) don't exist."),
+			     uid);
+	} else {
+		pw = getpwnam(info.username);
+		if (!pw)
+			errx(EXIT_FAILURE, _("user \"%s\" does not exist."),
+			     info.username);
 	}
-	while (fgets(buf, sizeof(buf), fp) != NULL) {
-		/* ignore comments */
-		if (*buf == '#')
-			continue;
-		len = strlen(buf);
-		/* strip the ending newline */
-		if (buf[len - 1] == '\n')
-			buf[len - 1] = 0;
-		/* ignore lines that are too damn long */
-		else
-			continue;
-		/* check or output the shell */
-		if (shell_name) {
-			if (!strcmp(shell_name, buf)) {
-				found = true;
-				break;
+
+#ifndef HAVE_LIBUSER
+	if (!(is_local(pw->pw_name)))
+		errx(EXIT_FAILURE, _("can only change local entries"));
+#endif
+
+#ifdef HAVE_LIBSELINUX
+	if (is_selinux_enabled() > 0) {
+		if (uid == 0) {
+			if (checkAccess(pw->pw_name, PASSWD__CHSH) != 0) {
+				security_context_t user_context;
+				if (getprevcon(&user_context) < 0)
+					user_context =
+					    (security_context_t) NULL;
+
+				errx(EXIT_FAILURE,
+				     _("%s is not authorized to change the shell of %s"),
+				     user_context ? : _("Unknown user context"),
+				     pw->pw_name);
 			}
-		} else
-			printf("%s\n", buf);
+		}
+		if (setupDefaultContext(_PATH_PASSWD) != 0)
+			errx(EXIT_FAILURE,
+			     _("can't set default context for %s"), _PATH_PASSWD);
 	}
-	fclose(fp);
-	return found;
+#endif
+
+	oldshell = pw->pw_shell;
+	if (oldshell == NULL || *oldshell == '\0')
+		oldshell = _PATH_BSHELL;	/* default */
+
+	/* reality check */
+#ifdef HAVE_LIBUSER
+	/* If we're setuid and not really root, disallow the password change. */
+	if (geteuid() != getuid() && uid != pw->pw_uid) {
+#else
+	if (uid != 0 && uid != pw->pw_uid) {
+#endif
+		errno = EACCES;
+		err(EXIT_FAILURE,
+		    _("running UID doesn't match UID of user we're "
+		      "altering, shell change denied"));
+	}
+	if (uid != 0 && !get_shell_list(oldshell)) {
+		errno = EACCES;
+		err(EXIT_FAILURE, _("your shell is not in %s, "
+				    "shell change denied"), _PATH_SHELLS);
+	}
+
+	shell = info.shell;
+
+	printf(_("Changing shell for %s.\n"), pw->pw_name);
+
+#if !defined(HAVE_LIBUSER) && defined(CHFN_CHSH_PASSWORD)
+	if(!auth_pam("chsh", uid, pw->pw_name)) {
+		return EXIT_FAILURE;
+	}
+#endif
+	if (!shell) {
+		shell = prompt(_("New shell"), oldshell);
+		if (!shell)
+			return EXIT_SUCCESS;
+	}
+
+	if (check_shell(shell) < 0)
+		return EXIT_FAILURE;
+
+	if (strcmp(oldshell, shell) == 0)
+		errx(EXIT_SUCCESS, _("Shell not changed."));
+
+#ifdef HAVE_LIBUSER
+	if (set_value_libuser("chsh", pw->pw_name, uid,
+	    LU_LOGINSHELL, shell) < 0)
+		errx(EXIT_FAILURE, _("Shell *NOT* changed.  Try again later."));
+#else
+	pw->pw_shell = shell;
+	if (setpwnam(pw) < 0)
+		err(EXIT_FAILURE, _("setpwnam failed\n"
+			"Shell *NOT* changed.  Try again later."));
+#endif
+
+	printf(_("Shell changed.\n"));
+	return EXIT_SUCCESS;
 }
-- 
2.2.1

[PATCH 02/14] chfn, chsh: share illegal_passwd_chars() function

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/Makemodule.am |  4 +++-
 login-utils/ch-common.c   | 34 ++++++++++++++++++++++++++++++++++
 login-utils/ch-common.h   |  6 ++++++
 login-utils/chfn.c        | 16 +++++-----------
 login-utils/chsh.c        | 18 +++++-------------
 5 files changed, 53 insertions(+), 25 deletions(-)
 create mode 100644 login-utils/ch-common.c
 create mode 100644 login-utils/ch-common.h

diff --git a/login-utils/Makemodule.am b/login-utils/Makemodule.am
index e1f88c3..f9c0977 100644
--- a/login-utils/Makemodule.am
+++ b/login-utils/Makemodule.am
@@ -83,7 +83,9 @@ dist_man_MANS += \
 	login-utils/chfn.1 \
 	login-utils/chsh.1
 
-chfn_chsh_sources =
+chfn_chsh_sources = \
+	login-utils/ch-common.h \
+	login-utils/ch-common.c
 chfn_chsh_cflags = $(SUID_CFLAGS) $(AM_CFLAGS)
 chfn_chsh_ldflags = $(SUID_LDFLAGS) $(AM_LDFLAGS)
 chfn_chsh_ldadd = libcommon.la
diff --git a/login-utils/ch-common.c b/login-utils/ch-common.c
new file mode 100644
index 0000000..34b09f3
--- /dev/null
+++ b/login-utils/ch-common.c
@@ -0,0 +1,34 @@
+/*
+ * chfn and chsh shared functions
+ *
+ * this program is free software.  you can redistribute it and
+ * modify it under the terms of the gnu general public license.
+ * there is no warranty.
+ */
+
+#include <ctype.h>
+#include <string.h>
+
+#include "c.h"
+#include "nls.h"
+
+#include "ch-common.h"
+
+/*
+ *  illegal_passwd_chars () -
+ *	check whether a string contains illegal characters
+ */
+int illegal_passwd_chars(const char *str)
+{
+	const char illegal[] = ",:=\"\n";
+	const size_t len = strlen(str);
+	size_t i;
+
+	if (strpbrk(str, illegal))
+		return 1;
+	for (i = 0; i < len; i++) {
+		if (iscntrl(str[i]))
+			return 1;
+	}
+	return 0;
+}
diff --git a/login-utils/ch-common.h b/login-utils/ch-common.h
new file mode 100644
index 0000000..7f70e50
--- /dev/null
+++ b/login-utils/ch-common.h
@@ -0,0 +1,6 @@
+#ifndef UTIL_LINUX_CH_COMMON_H
+#define UTIL_LINUX_CH_COMMON_H
+
+extern int illegal_passwd_chars(const char *str);
+
+#endif	/* UTIL_LINUX_CH_COMMON */
diff --git a/login-utils/chfn.c b/login-utils/chfn.c
index 15f897c..2f1b70d 100644
--- a/login-utils/chfn.c
+++ b/login-utils/chfn.c
@@ -42,6 +42,8 @@
 #include "xalloc.h"
 #include "logindefs.h"
 
+#include "ch-common.h"
+
 #ifdef HAVE_LIBSELINUX
 # include <selinux/selinux.h>
 # include <selinux/av_permissions.h>
@@ -106,23 +108,15 @@ static void __attribute__((__noreturn__)) usage(FILE *fp)
  */
 static int check_gecos_string(const char *msg, char *gecos)
 {
-	unsigned int i, c;
 	const size_t len = strlen(gecos);
 
 	if (MAX_FIELD_SIZE < len) {
 		warnx(_("field %s is too long"), msg);
 		return -1;
 	}
-	for (i = 0; i < len; i++) {
-		c = gecos[i];
-		if (c == ',' || c == ':' || c == '=' || c == '"' || c == '\n') {
-			warnx(_("%s: '%c' is not allowed"), msg, c);
-			return -1;
-		}
-		if (iscntrl(c)) {
-			warnx(_("%s: control characters are not allowed"), msg);
-			return -1;
-		}
+	if (illegal_passwd_chars(gecos)) {
+		warnx(_("%s: has illegal characters"), gecos);
+		return -1;
 	}
 	return 0;
 }
diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index 1327614..2245f41 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -42,6 +42,8 @@
 #include "setpwnam.h"
 #include "xalloc.h"
 
+#include "ch-common.h"
+
 #ifdef HAVE_LIBSELINUX
 # include <selinux/selinux.h>
 # include <selinux/av_permissions.h>
@@ -205,8 +207,6 @@ static char *prompt(char *question, char *def_val)
  */
 static int check_shell(char *shell)
 {
-	unsigned int i, c;
-
 	if (!shell)
 		return -1;
 
@@ -222,17 +222,9 @@ static int check_shell(char *shell)
 		printf(_("\"%s\" is not executable"), shell);
 		return -1;
 	}
-	/* keep /etc/passwd clean. */
-	for (i = 0; i < strlen(shell); i++) {
-		c = shell[i];
-		if (c == ',' || c == ':' || c == '=' || c == '"' || c == '\n') {
-			warnx(_("'%c' is not allowed"), c);
-			return -1;
-		}
-		if (iscntrl(c)) {
-			warnx(_("control characters are not allowed"));
-			return -1;
-		}
+	if (illegal_passwd_chars(shell)) {
+		warnx(_("%s: has illegal characters"), shell);
+		return -1;
 	}
 #ifdef ONLY_LISTED_SHELLS
 	if (!get_shell_list(shell)) {
-- 
2.2.1

[PATCH 03/14] chsh: use getline() to support arbitrarily long lines

[Sami Kerola]

Use of fgets() can make a single long line to be understood as two
entries, and someone could play tricks with the remainder part of the
buffer.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/chsh.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index 2245f41..9ed4d06 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -87,8 +87,8 @@ static int get_shell_list(char *shell_name)
 {
 	FILE *fp;
 	int found;
-	int len;
-	char buf[PATH_MAX];
+	char *buf = NULL;
+	size_t sz = 0, len;
 
 	found = false;
 	fp = fopen(_PATH_SHELLS, "r");
@@ -97,17 +97,17 @@ static int get_shell_list(char *shell_name)
 			warnx(_("No known shells."));
 		return true;
 	}
-	while (fgets(buf, sizeof(buf), fp) != NULL) {
+	while (getline(&buf, &sz, fp) != -1) {
+		len = strlen(buf);
 		/* ignore comments */
 		if (*buf == '#')
 			continue;
-		len = strlen(buf);
+		/* skip blank lines*/
+		if (len < 2)
+			continue;
 		/* strip the ending newline */
 		if (buf[len - 1] == '\n')
 			buf[len - 1] = 0;
-		/* ignore lines that are too damn long */
-		else
-			continue;
 		/* check or output the shell */
 		if (shell_name) {
 			if (!strcmp(shell_name, buf)) {
@@ -118,6 +118,7 @@ static int get_shell_list(char *shell_name)
 			printf("%s\n", buf);
 	}
 	fclose(fp);
+	free(buf);
 	return found;
 }
 
-- 
2.2.1

[PATCH 04/14] chsh: set few variables read-only and rename one of them

[Sami Kerola]

This change also improves couple variable initializations.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/chsh.c | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index 9ed4d06..a316415 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -83,7 +83,7 @@ static void __attribute__((__noreturn__)) usage (FILE *fp)
  *	return true.  if not, return false.
  *	if the given shell is NULL, /etc/shells is outputted to stdout.
  */
-static int get_shell_list(char *shell_name)
+static int get_shell_list(const char *shell_name)
 {
 	FILE *fp;
 	int found;
@@ -206,7 +206,7 @@ static char *prompt(char *question, char *def_val)
  *	an error and return (-1).
  *	if the shell is a bad idea, print a warning.
  */
-static int check_shell(char *shell)
+static int check_shell(const char *shell)
 {
 	if (!shell)
 		return -1;
@@ -251,9 +251,9 @@ static int check_shell(char *shell)
 
 int main(int argc, char **argv)
 {
-	char *shell, *oldshell;
-	uid_t uid;
-	struct sinfo info;
+	char *oldshell;
+	const uid_t uid = getuid();
+	struct sinfo info = { 0 };
 	struct passwd *pw;
 
 	sanitize_env();
@@ -262,11 +262,7 @@ int main(int argc, char **argv)
 	textdomain(PACKAGE);
 	atexit(close_stdout);
 
-	uid = getuid();
-	memset(&info, 0, sizeof(info));
-
 	parse_argv(argc, argv, &info);
-	pw = NULL;
 	if (!info.username) {
 		pw = getpwuid(uid);
 		if (!pw)
@@ -327,33 +323,31 @@ int main(int argc, char **argv)
 				    "shell change denied"), _PATH_SHELLS);
 	}
 
-	shell = info.shell;
-
 	printf(_("Changing shell for %s.\n"), pw->pw_name);
 
 #if !defined(HAVE_LIBUSER) && defined(CHFN_CHSH_PASSWORD)
-	if(!auth_pam("chsh", uid, pw->pw_name)) {
+	if (!auth_pam("chsh", uid, pw->pw_name)) {
 		return EXIT_FAILURE;
 	}
 #endif
-	if (!shell) {
-		shell = prompt(_("New shell"), oldshell);
-		if (!shell)
+	if (!info.shell) {
+		info.shell = prompt(_("New shell"), oldshell);
+		if (!info.shell)
 			return EXIT_SUCCESS;
 	}
 
-	if (check_shell(shell) < 0)
+	if (check_shell(info.shell) < 0)
 		return EXIT_FAILURE;
 
-	if (strcmp(oldshell, shell) == 0)
+	if (strcmp(oldshell, info.shell) == 0)
 		errx(EXIT_SUCCESS, _("Shell not changed."));
 
 #ifdef HAVE_LIBUSER
 	if (set_value_libuser("chsh", pw->pw_name, uid,
-	    LU_LOGINSHELL, shell) < 0)
+	    LU_LOGINSHELL, info.shell) < 0)
 		errx(EXIT_FAILURE, _("Shell *NOT* changed.  Try again later."));
 #else
-	pw->pw_shell = shell;
+	pw->pw_shell = info.shell;
 	if (setpwnam(pw) < 0)
 		err(EXIT_FAILURE, _("setpwnam failed\n"
 			"Shell *NOT* changed.  Try again later."));
-- 
2.2.1

[PATCH 05/14] chsh: allow user to set shell to /bin/sh if none is set

[Sami Kerola]

Earlier setting a /bin/sh was impossible for users that had nothing set
as shell, as that was seen as no change.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/chsh.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index a316415..0112fa4 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -252,6 +252,7 @@ static int check_shell(const char *shell)
 int main(int argc, char **argv)
 {
 	char *oldshell;
+	int nullshell = 0;
 	const uid_t uid = getuid();
 	struct sinfo info = { 0 };
 	struct passwd *pw;
@@ -302,8 +303,10 @@ int main(int argc, char **argv)
 #endif
 
 	oldshell = pw->pw_shell;
-	if (oldshell == NULL || *oldshell == '\0')
+	if (oldshell == NULL || *oldshell == '\0') {
 		oldshell = _PATH_BSHELL;	/* default */
+		nullshell = 1;
+	}
 
 	/* reality check */
 #ifdef HAVE_LIBUSER
@@ -339,7 +342,7 @@ int main(int argc, char **argv)
 	if (check_shell(info.shell) < 0)
 		return EXIT_FAILURE;
 
-	if (strcmp(oldshell, info.shell) == 0)
+	if (!nullshell && strcmp(oldshell, info.shell) == 0)
 		errx(EXIT_SUCCESS, _("Shell not changed."));
 
 #ifdef HAVE_LIBUSER
-- 
2.2.1

[PATCH 06/14] chsh: clean up parse_argv()

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/chsh.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index 0112fa4..5339e55 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -129,22 +129,17 @@ static int get_shell_list(const char *shell_name)
  */
 static void parse_argv(int argc, char **argv, struct sinfo *pinfo)
 {
-	int index, c;
-
-	static struct option long_options[] = {
+	const struct option long_options[] = {
 		{"shell", required_argument, 0, 's'},
 		{"list-shells", no_argument, 0, 'l'},
 		{"help", no_argument, 0, 'u'},
 		{"version", no_argument, 0, 'v'},
 		{NULL, no_argument, 0, '0'},
 	};
+	int c;
 
-	optind = c = 0;
-	while (c != EOF) {
-		c = getopt_long(argc, argv, "s:luv", long_options, &index);
+	while ((c = getopt_long(argc, argv, "s:luv", long_options, NULL)) != -1) {
 		switch (c) {
-		case -1:
-			break;
 		case 'v':
 			printf(UTIL_LINUX_VERSION);
 			exit(EXIT_SUCCESS);
-- 
2.2.1

[PATCH 07/14] chsh: rewrite function interacting with user to get path to new shell

[Sami Kerola]

Rename prompt() to ask_new_shell().  Remove fixed size buffer and
allocate path to new shell, that should make Hurd people happy.  Use
strutils.h for white space trimming.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/chsh.c | 43 +++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index 5339e55..bcd0995 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -40,6 +40,7 @@
 #include "nls.h"
 #include "pathnames.h"
 #include "setpwnam.h"
+#include "strutils.h"
 #include "xalloc.h"
 
 #include "ch-common.h"
@@ -166,34 +167,28 @@ static void parse_argv(int argc, char **argv, struct sinfo *pinfo)
 }
 
 /*
- *  prompt () --
- *	ask the user for a given field and return it.
+ *  ask_new_shell () --
+ *	ask the user for a shell and return it.
  */
-static char *prompt(char *question, char *def_val)
+static char *ask_new_shell(char *question, char *oldshell)
 {
 	int len;
-	char *ans, *cp;
-	char buf[BUFSIZ];
+	char *ans = NULL;
+	size_t dummy = 0;
+	ssize_t sz;
 
-	if (!def_val)
-		def_val = "";
-	printf("%s [%s]: ", question, def_val);
-	*buf = 0;
-	if (fgets(buf, sizeof(buf), stdin) == NULL)
-		errx(EXIT_FAILURE, _("Aborted."));
-	/* remove the newline at the end of buf. */
-	ans = buf;
-	while (isspace(*ans))
-		ans++;
-	len = strlen(ans);
-	while (len > 0 && isspace(ans[len - 1]))
-		len--;
-	if (len <= 0)
+	if (!oldshell)
+		oldshell = "";
+	printf("%s [%s]: ", question, oldshell);
+	sz = getline(&ans, &dummy, stdin);
+	if (sz == -1)
 		return NULL;
-	ans[len] = 0;
-	cp = (char *)xmalloc(len + 1);
-	strcpy(cp, ans);
-	return cp;
+	/* remove the newline at the end of ans. */
+	ltrim_whitespace((unsigned char *) ans);
+	len = rtrim_whitespace((unsigned char *) ans);
+	if (len == 0)
+		return NULL;
+	return ans;
 }
 
 /*
@@ -329,7 +324,7 @@ int main(int argc, char **argv)
 	}
 #endif
 	if (!info.shell) {
-		info.shell = prompt(_("New shell"), oldshell);
+		info.shell = ask_new_shell(_("New shell"), oldshell);
 		if (!info.shell)
 			return EXIT_SUCCESS;
 	}
-- 
2.2.1

[PATCH 08/14] chsh: simplify check_shell()

[Sami Kerola]

Shell null check is redundant.  The shell can be null only after
ask_new_shell returned such, and that is checked earlier in program
logic.

Secondly the check_shell does not need to return values, in such cases
the program can simply exit.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/chsh.c | 49 ++++++++++++++++---------------------------------
 1 file changed, 16 insertions(+), 33 deletions(-)

diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index bcd0995..3c51783 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -193,50 +193,34 @@ static char *ask_new_shell(char *question, char *oldshell)
 
 /*
  *  check_shell () -- if the shell is completely invalid, print
- *	an error and return (-1).
- *	if the shell is a bad idea, print a warning.
+ *	an error and exit.
  */
-static int check_shell(const char *shell)
+static void check_shell(const char *shell)
 {
-	if (!shell)
-		return -1;
-
-	if (*shell != '/') {
-		warnx(_("shell must be a full path name"));
-		return -1;
-	}
-	if (access(shell, F_OK) < 0) {
-		warnx(_("\"%s\" does not exist"), shell);
-		return -1;
-	}
-	if (access(shell, X_OK) < 0) {
-		printf(_("\"%s\" is not executable"), shell);
-		return -1;
-	}
-	if (illegal_passwd_chars(shell)) {
-		warnx(_("%s: has illegal characters"), shell);
-		return -1;
-	}
-#ifdef ONLY_LISTED_SHELLS
+	if (*shell != '/')
+		errx(EXIT_FAILURE, _("shell must be a full path name"));
+	if (access(shell, F_OK) < 0)
+		errx(EXIT_FAILURE, _("\"%s\" does not exist"), shell);
+	if (access(shell, X_OK) < 0)
+		errx(EXIT_FAILURE, _("\"%s\" is not executable"), shell);
+	if (illegal_passwd_chars(shell))
+		errx(EXIT_FAILURE, _("%s: has illegal characters"), shell);
 	if (!get_shell_list(shell)) {
+#ifdef ONLY_LISTED_SHELLS
 		if (!getuid())
-			warnx(_
-			      ("Warning: \"%s\" is not listed in %s."),
-			      shell, _PATH_SHELLS);
+			warnx(_("Warning: \"%s\" is not listed in %s."), shell,
+			      _PATH_SHELLS);
 		else
 			errx(EXIT_FAILURE,
 			     _("\"%s\" is not listed in %s.\n"
 			       "Use %s -l to see list."), shell, _PATH_SHELLS,
 			     program_invocation_short_name);
-	}
 #else
-	if (!get_shell_list(shell)) {
 		warnx(_("\"%s\" is not listed in %s.\n"
 			"Use %s -l to see list."), shell, _PATH_SHELLS,
-		      program_invocation_short_name);
-	}
+		       program_invocation_short_name);
 #endif
-	return 0;
+	}
 }
 
 int main(int argc, char **argv)
@@ -329,8 +313,7 @@ int main(int argc, char **argv)
 			return EXIT_SUCCESS;
 	}
 
-	if (check_shell(info.shell) < 0)
-		return EXIT_FAILURE;
+	check_shell(info.shell);
 
 	if (!nullshell && strcmp(oldshell, info.shell) == 0)
 		errx(EXIT_SUCCESS, _("Shell not changed."));
-- 
2.2.1

[PATCH 09/14] chsh: fail get_shell_list() check when /etc/shells cannot be opened

[Sami Kerola]

And get rid of stdbool.h true/false usage.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/chsh.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index 3c51783..a84e3ff 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -26,7 +26,6 @@
 #include <errno.h>
 #include <getopt.h>
 #include <pwd.h>
-#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -87,16 +86,15 @@ static void __attribute__((__noreturn__)) usage (FILE *fp)
 static int get_shell_list(const char *shell_name)
 {
 	FILE *fp;
-	int found;
+	int found = 0;
 	char *buf = NULL;
 	size_t sz = 0, len;
 
-	found = false;
 	fp = fopen(_PATH_SHELLS, "r");
 	if (!fp) {
 		if (!shell_name)
 			warnx(_("No known shells."));
-		return true;
+		return 0;
 	}
 	while (getline(&buf, &sz, fp) != -1) {
 		len = strlen(buf);
@@ -112,7 +110,7 @@ static int get_shell_list(const char *shell_name)
 		/* check or output the shell */
 		if (shell_name) {
 			if (!strcmp(shell_name, buf)) {
-				found = true;
+				found = 1;
 				break;
 			}
 		} else
-- 
2.2.1

[PATCH 10/14] newgrp: simplify if else clauses

[Sami Kerola]

The 'if' clauses that have termination as either of the control flow
results will never need 'else'.  Making the termination to happen true
flow is enough.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/newgrp.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/login-utils/newgrp.c b/login-utils/newgrp.c
index 58c9078..3f5c720 100644
--- a/login-utils/newgrp.c
+++ b/login-utils/newgrp.c
@@ -214,15 +214,12 @@ int main(int argc, char *argv[])
 			if (errno)
 				err(EXIT_FAILURE, _("no such group"));
 			else
-				/* No group */
 				errx(EXIT_FAILURE, _("no such group"));
-		} else {
-			if (allow_setgid(pw_entry, gr_entry)) {
-				if (setgid(gr_entry->gr_gid) < 0)
-					err(EXIT_FAILURE, _("setgid failed"));
-			} else
-				errx(EXIT_FAILURE, _("permission denied"));
 		}
+		if (!allow_setgid(pw_entry, gr_entry))
+			errx(EXIT_FAILURE, _("permission denied"));
+		if (setgid(gr_entry->gr_gid) < 0)
+			err(EXIT_FAILURE, _("setgid failed"));
 	}
 
 	if (setuid(getuid()) < 0)
-- 
2.2.1

[PATCH 11/14] newgrp: move shell determination closer where it is used

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/newgrp.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/login-utils/newgrp.c b/login-utils/newgrp.c
index 3f5c720..0fae087 100644
--- a/login-utils/newgrp.c
+++ b/login-utils/newgrp.c
@@ -202,9 +202,6 @@ int main(int argc, char *argv[])
 	if (!(pw_entry = getpwuid(getuid())))
 		err(EXIT_FAILURE, _("who are you?"));
 
-	shell = (pw_entry->pw_shell && *pw_entry->pw_shell ?
-				pw_entry->pw_shell : _PATH_BSHELL);
-
 	if (argc < 2) {
 		if (setgid(pw_entry->pw_gid) < 0)
 			err(EXIT_FAILURE, _("setgid failed"));
@@ -225,8 +222,9 @@ int main(int argc, char *argv[])
 	if (setuid(getuid()) < 0)
 		err(EXIT_FAILURE, _("setuid failed"));
 
-	fflush(stdout);
-	fflush(stderr);
+	fflush(NULL);
+	shell = (pw_entry->pw_shell && *pw_entry->pw_shell ?
+				pw_entry->pw_shell : _PATH_BSHELL);
 	execl(shell, shell, (char *)0);
 	warn(_("failed to execute %s"), shell);
 	fflush(stderr);
-- 
2.2.1

[PATCH 12/14] newgrp: set function arguments read-only when possible

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/newgrp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/login-utils/newgrp.c b/login-utils/newgrp.c
index 0fae087..b1134e9 100644
--- a/login-utils/newgrp.c
+++ b/login-utils/newgrp.c
@@ -74,7 +74,7 @@ static int memset_s(void *v, size_t sz, const int c)
 }
 
 /* try to read password from gshadow */
-static char *get_gshadow_pwd(char *groupname)
+static char *get_gshadow_pwd(const char *groupname)
 {
 #ifdef HAVE_GETSGNAM
 	struct sgrp *sgrp;
@@ -117,7 +117,7 @@ static char *get_gshadow_pwd(char *groupname)
 #endif	/* HAVE_GETSGNAM */
 }
 
-static int allow_setgid(struct passwd *pe, struct group *ge)
+static int allow_setgid(const struct passwd *pe, const struct group *ge)
 {
 	char **look;
 	int notfound = 1;
@@ -160,7 +160,7 @@ static int allow_setgid(struct passwd *pe, struct group *ge)
 	return FALSE;
 }
 
-static void __attribute__ ((__noreturn__)) usage(FILE * out)
+static void __attribute__((__noreturn__)) usage(FILE *out)
 {
 	fprintf(out, USAGE_HEADER);
 	fprintf(out, _(" %s <group>\n"), program_invocation_short_name);
-- 
2.2.1

[PATCH 13/14] script: remove function prototypes

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 term-utils/script.c | 497 +++++++++++++++++++++++++---------------------------
 1 file changed, 242 insertions(+), 255 deletions(-)

diff --git a/term-utils/script.c b/term-utils/script.c
index 78d7a30..0eaa73f 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -80,18 +80,6 @@
 
 #define DEFAULT_OUTPUT "typescript"
 
-void sig_finish(int);
-void finish(int);
-void done(void);
-void fail(void);
-void resize(int);
-void fixtty(void);
-void getmaster(void);
-void getslave(void);
-void doinput(void);
-void dooutput(void);
-void doshell(void);
-
 char	*shell;
 FILE	*fscript;
 FILE	*timingfd;
@@ -123,21 +111,7 @@ sigset_t block_mask, unblock_mask;
 int die;
 int resized;
 
-static void
-die_if_link(char *fn) {
-	struct stat s;
-
-	if (forceflg)
-		return;
-	if (lstat(fn, &s) == 0 && (S_ISLNK(s.st_mode) || s.st_nlink > 1))
-		errx(EXIT_FAILURE,
-		     _("output file `%s' is a link\n"
-		       "Use --force if you really want to use it.\n"
-		       "Program not started."), fn);
-}
-
-static void __attribute__((__noreturn__))
-usage(FILE *out)
+static void __attribute__((__noreturn__)) usage(FILE *out)
 {
 	fputs(USAGE_HEADER, out);
 	fprintf(out,
@@ -158,145 +132,81 @@ usage(FILE *out)
 	exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
 }
 
-/*
- * script -t prints time delays as floating point numbers
- * The example program (scriptreplay) that we provide to handle this
- * timing output is a perl script, and does not handle numbers in
- * locale format (not even when "use locale;" is added).
- * So, since these numbers are not for human consumption, it seems
- * easiest to set LC_NUMERIC here.
- */
-
-int
-main(int argc, char **argv) {
-	struct sigaction sa;
-	int ch;
+static void die_if_link(char *fn)
+{
+	struct stat s;
 
-	enum { FORCE_OPTION = CHAR_MAX + 1 };
+	if (forceflg)
+		return;
+	if (lstat(fn, &s) == 0 && (S_ISLNK(s.st_mode) || s.st_nlink > 1))
+		errx(EXIT_FAILURE,
+		     _("output file `%s' is a link\n"
+		       "Use --force if you really want to use it.\n"
+		       "Program not started."), fn);
+}
 
-	static const struct option longopts[] = {
-		{ "append",	no_argument,	   NULL, 'a' },
-		{ "command",	required_argument, NULL, 'c' },
-		{ "return",	no_argument,	   NULL, 'e' },
-		{ "flush",	no_argument,	   NULL, 'f' },
-		{ "force",	no_argument,	   NULL, FORCE_OPTION, },
-		{ "quiet",	no_argument,	   NULL, 'q' },
-		{ "timing",	optional_argument, NULL, 't' },
-		{ "version",	no_argument,	   NULL, 'V' },
-		{ "help",	no_argument,	   NULL, 'h' },
-		{ NULL,		0, NULL, 0 }
-	};
+/*
+ * Stop extremely silly gcc complaint on %c:
+ *  warning: `%c' yields only last 2 digits of year in some locales
+ */
+static void my_strftime(char *buf, size_t len, const char *fmt, const struct tm *tm)
+{
+	strftime(buf, len, fmt, tm);
+}
 
-	setlocale(LC_ALL, "");
-	setlocale(LC_NUMERIC, "C");	/* see comment above */
-	bindtextdomain(PACKAGE, LOCALEDIR);
-	textdomain(PACKAGE);
-	atexit(close_stdout);
+static void __attribute__((__noreturn__)) done(void)
+{
+	time_t tvec;
 
-	while ((ch = getopt_long(argc, argv, "ac:efqt::Vh", longopts, NULL)) != -1)
-		switch(ch) {
-		case 'a':
-			aflg = 1;
-			break;
-		case 'c':
-			cflg = optarg;
-			break;
-		case 'e':
-			eflg = 1;
-			break;
-		case 'f':
-			fflg = 1;
-			break;
-		case FORCE_OPTION:
-			forceflg = 1;
-			break;
-		case 'q':
-			qflg = 1;
-			break;
-		case 't':
-			if (optarg && !(timingfd = fopen(optarg, "w")))
-				err(EXIT_FAILURE, _("cannot open %s"), optarg);
-			tflg = 1;
-			break;
-		case 'V':
-			printf(UTIL_LINUX_VERSION);
-			exit(EXIT_SUCCESS);
-			break;
-		case 'h':
-			usage(stdout);
-			break;
-		case '?':
-		default:
-			usage(stderr);
+	if (subchild) {
+		/* output process */
+		if (fscript) {
+			if (!qflg) {
+				char buf[BUFSIZ];
+				tvec = time((time_t *)NULL);
+				my_strftime(buf, sizeof buf, "%c\n", localtime(&tvec));
+				fprintf(fscript, _("\nScript done on %s"), buf);
+			}
+			if (close_stream(fscript) != 0)
+				errx(EXIT_FAILURE, _("write error"));
+			fscript = NULL;
 		}
-	argc -= optind;
-	argv += optind;
-
-	if (argc > 0)
-		fname = argv[0];
-	else {
-		fname = DEFAULT_OUTPUT;
-		die_if_link(fname);
-	}
-
-	if ((fscript = fopen(fname, aflg ? "a" : "w")) == NULL) {
-		warn(_("cannot open %s"), fname);
-		fail();
-	}
-
-	shell = getenv("SHELL");
-	if (shell == NULL)
-		shell = _PATH_BSHELL;
-
-	getmaster();
-	if (!qflg)
-		printf(_("Script started, file is %s\n"), fname);
-	fixtty();
+		if (timingfd && close_stream(timingfd) != 0)
+			errx(EXIT_FAILURE, _("write error"));
+		timingfd = NULL;
 
+		close(master);
+		master = -1;
+	} else {
+		/* input process */
+		if (isterm)
+			tcsetattr(STDIN_FILENO, TCSADRAIN, &tt);
+		if (!qflg)
+			printf(_("Script done, file is %s\n"), fname);
 #ifdef HAVE_LIBUTEMPTER
-	utempter_add_record(master, NULL);
+		if (master >= 0)
+			utempter_remove_record(master);
 #endif
-	/* setup SIGCHLD handler */
-	sigemptyset(&sa.sa_mask);
-	sa.sa_flags = 0;
-	sa.sa_handler = sig_finish;
-	sigaction(SIGCHLD, &sa, NULL);
-
-	/* init mask for SIGCHLD */
-	sigprocmask(SIG_SETMASK, NULL, &block_mask);
-	sigaddset(&block_mask, SIGCHLD);
-
-	sigprocmask(SIG_SETMASK, &block_mask, &unblock_mask);
-	child = fork();
-	sigprocmask(SIG_SETMASK, &unblock_mask, NULL);
-
-	if (child < 0) {
-		warn(_("fork failed"));
-		fail();
+		kill(child, SIGTERM);	/* make sure we don't create orphans */
 	}
-	if (child == 0) {
-
-		sigprocmask(SIG_SETMASK, &block_mask, NULL);
-		subchild = child = fork();
-		sigprocmask(SIG_SETMASK, &unblock_mask, NULL);
 
-		if (child < 0) {
-			warn(_("fork failed"));
-			fail();
-		}
-		if (child)
-			dooutput();
+	if(eflg) {
+		if (WIFSIGNALED(childstatus))
+			exit(WTERMSIG(childstatus) + 0x80);
 		else
-			doshell();
-	} else {
-		sa.sa_handler = resize;
-		sigaction(SIGWINCH, &sa, NULL);
+			exit(WEXITSTATUS(childstatus));
 	}
-	doinput();
+	exit(EXIT_SUCCESS);
+}
 
-	return EXIT_SUCCESS;
+static void
+fail(void) {
+
+	kill(0, SIGTERM);
+	done();
 }
 
+
 static void wait_for_empty_fd(int fd)
 {
 	struct pollfd fds[] = {
@@ -306,8 +216,24 @@ static void wait_for_empty_fd(int fd)
 	while (die == 0 && poll(fds, 1, 100) == 1);
 }
 
-void
-doinput(void) {
+static void finish(int wait)
+{
+	int status;
+	pid_t pid;
+	int errsv = errno;
+	int options = wait ? 0 : WNOHANG;
+
+	while ((pid = wait3(&status, options, 0)) > 0)
+		if (pid == child) {
+			childstatus = status;
+			die = 1;
+		}
+
+	errno = errsv;
+}
+
+static void doinput(void)
+{
 	int errsv = 0;
 	ssize_t cc = 0;
 	char ibuf[BUFSIZ];
@@ -390,43 +316,18 @@ doinput(void) {
 	done();
 }
 
-void
-finish(int wait) {
-	int status;
-	pid_t pid;
-	int errsv = errno;
-	int options = wait ? 0 : WNOHANG;
-
-	while ((pid = wait3(&status, options, 0)) > 0)
-		if (pid == child) {
-			childstatus = status;
-			die = 1;
-		}
-
-	errno = errsv;
-}
-
-void
-sig_finish(int dummy __attribute__ ((__unused__))) {
+static void sig_finish(int dummy __attribute__ ((__unused__)))
+{
 	finish(0);
 }
 
-void
-resize(int dummy __attribute__ ((__unused__))) {
+static void resize(int dummy __attribute__ ((__unused__)))
+{
 	resized = 1;
 }
 
-/*
- * Stop extremely silly gcc complaint on %c:
- *  warning: `%c' yields only last 2 digits of year in some locales
- */
-static void
-my_strftime(char *buf, size_t len, const char *fmt, const struct tm *tm) {
-	strftime(buf, len, fmt, tm);
-}
-
-void
-dooutput(void) {
+static void dooutput(void)
+{
 	ssize_t cc;
 	char obuf[BUFSIZ];
 	struct timeval tv;
@@ -503,8 +404,26 @@ dooutput(void) {
 	done();
 }
 
-void
-doshell(void) {
+static void getslave(void)
+{
+#ifndef HAVE_LIBUTIL
+	line[strlen("/dev/")] = 't';
+	slave = open(line, O_RDWR);
+	if (slave < 0) {
+		warn(_("cannot open %s"), line);
+		fail();
+	}
+	if (isterm) {
+		tcsetattr(slave, TCSANOW, &tt);
+		ioctl(slave, TIOCSWINSZ, (char *)&win);
+	}
+#endif
+	setsid();
+	ioctl(slave, TIOCSCTTY, 0);
+}
+
+static void doshell(void)
+{
 	char *shname;
 
 	getslave();
@@ -555,8 +474,8 @@ doshell(void) {
 	fail();
 }
 
-void
-fixtty(void) {
+static void fixtty(void)
+{
 	struct termios rtt;
 
 	if (!isterm)
@@ -568,60 +487,8 @@ fixtty(void) {
 	tcsetattr(STDIN_FILENO, TCSANOW, &rtt);
 }
 
-void
-fail(void) {
-
-	kill(0, SIGTERM);
-	done();
-}
-
-void __attribute__((__noreturn__))
-done(void) {
-	time_t tvec;
-
-	if (subchild) {
-		/* output process */
-		if (fscript) {
-			if (!qflg) {
-				char buf[BUFSIZ];
-				tvec = time((time_t *)NULL);
-				my_strftime(buf, sizeof buf, "%c\n", localtime(&tvec));
-				fprintf(fscript, _("\nScript done on %s"), buf);
-			}
-			if (close_stream(fscript) != 0)
-				errx(EXIT_FAILURE, _("write error"));
-			fscript = NULL;
-		}
-		if (timingfd && close_stream(timingfd) != 0)
-			errx(EXIT_FAILURE, _("write error"));
-		timingfd = NULL;
-
-		close(master);
-		master = -1;
-	} else {
-		/* input process */
-		if (isterm)
-			tcsetattr(STDIN_FILENO, TCSADRAIN, &tt);
-		if (!qflg)
-			printf(_("Script done, file is %s\n"), fname);
-#ifdef HAVE_LIBUTEMPTER
-		if (master >= 0)
-			utempter_remove_record(master);
-#endif
-		kill(child, SIGTERM);	/* make sure we don't create orphans */
-	}
-
-	if(eflg) {
-		if (WIFSIGNALED(childstatus))
-			exit(WTERMSIG(childstatus) + 0x80);
-		else
-			exit(WEXITSTATUS(childstatus));
-	}
-	exit(EXIT_SUCCESS);
-}
-
-void
-getmaster(void) {
+static void getmaster(void)
+{
 #if defined(HAVE_LIBUTIL) && defined(HAVE_PTY_H)
 	int rc;
 
@@ -681,20 +548,140 @@ getmaster(void) {
 #endif /* not HAVE_LIBUTIL */
 }
 
-void
-getslave(void) {
-#ifndef HAVE_LIBUTIL
-	line[strlen("/dev/")] = 't';
-	slave = open(line, O_RDWR);
-	if (slave < 0) {
-		warn(_("cannot open %s"), line);
-		fail();
+/*
+ * script -t prints time delays as floating point numbers
+ * The example program (scriptreplay) that we provide to handle this
+ * timing output is a perl script, and does not handle numbers in
+ * locale format (not even when "use locale;" is added).
+ * So, since these numbers are not for human consumption, it seems
+ * easiest to set LC_NUMERIC here.
+ */
+int main(int argc, char **argv)
+{
+	struct sigaction sa;
+	int ch;
+
+	enum { FORCE_OPTION = CHAR_MAX + 1 };
+
+	static const struct option longopts[] = {
+		{ "append",	no_argument,	   NULL, 'a' },
+		{ "command",	required_argument, NULL, 'c' },
+		{ "return",	no_argument,	   NULL, 'e' },
+		{ "flush",	no_argument,	   NULL, 'f' },
+		{ "force",	no_argument,	   NULL, FORCE_OPTION, },
+		{ "quiet",	no_argument,	   NULL, 'q' },
+		{ "timing",	optional_argument, NULL, 't' },
+		{ "version",	no_argument,	   NULL, 'V' },
+		{ "help",	no_argument,	   NULL, 'h' },
+		{ NULL,		0, NULL, 0 }
+	};
+
+	setlocale(LC_ALL, "");
+	setlocale(LC_NUMERIC, "C");	/* see comment above */
+	bindtextdomain(PACKAGE, LOCALEDIR);
+	textdomain(PACKAGE);
+	atexit(close_stdout);
+
+	while ((ch = getopt_long(argc, argv, "ac:efqt::Vh", longopts, NULL)) != -1)
+		switch(ch) {
+		case 'a':
+			aflg = 1;
+			break;
+		case 'c':
+			cflg = optarg;
+			break;
+		case 'e':
+			eflg = 1;
+			break;
+		case 'f':
+			fflg = 1;
+			break;
+		case FORCE_OPTION:
+			forceflg = 1;
+			break;
+		case 'q':
+			qflg = 1;
+			break;
+		case 't':
+			if (optarg && !(timingfd = fopen(optarg, "w")))
+				err(EXIT_FAILURE, _("cannot open %s"), optarg);
+			tflg = 1;
+			break;
+		case 'V':
+			printf(UTIL_LINUX_VERSION);
+			exit(EXIT_SUCCESS);
+			break;
+		case 'h':
+			usage(stdout);
+			break;
+		case '?':
+		default:
+			usage(stderr);
+		}
+	argc -= optind;
+	argv += optind;
+
+	if (argc > 0)
+		fname = argv[0];
+	else {
+		fname = DEFAULT_OUTPUT;
+		die_if_link(fname);
 	}
-	if (isterm) {
-		tcsetattr(slave, TCSANOW, &tt);
-		ioctl(slave, TIOCSWINSZ, (char *)&win);
+
+	if ((fscript = fopen(fname, aflg ? "a" : "w")) == NULL) {
+		warn(_("cannot open %s"), fname);
+		fail();
 	}
+
+	shell = getenv("SHELL");
+	if (shell == NULL)
+		shell = _PATH_BSHELL;
+
+	getmaster();
+	if (!qflg)
+		printf(_("Script started, file is %s\n"), fname);
+	fixtty();
+
+#ifdef HAVE_LIBUTEMPTER
+	utempter_add_record(master, NULL);
 #endif
-	setsid();
-	ioctl(slave, TIOCSCTTY, 0);
+	/* setup SIGCHLD handler */
+	sigemptyset(&sa.sa_mask);
+	sa.sa_flags = 0;
+	sa.sa_handler = sig_finish;
+	sigaction(SIGCHLD, &sa, NULL);
+
+	/* init mask for SIGCHLD */
+	sigprocmask(SIG_SETMASK, NULL, &block_mask);
+	sigaddset(&block_mask, SIGCHLD);
+
+	sigprocmask(SIG_SETMASK, &block_mask, &unblock_mask);
+	child = fork();
+	sigprocmask(SIG_SETMASK, &unblock_mask, NULL);
+
+	if (child < 0) {
+		warn(_("fork failed"));
+		fail();
+	}
+	if (child == 0) {
+
+		sigprocmask(SIG_SETMASK, &block_mask, NULL);
+		subchild = child = fork();
+		sigprocmask(SIG_SETMASK, &unblock_mask, NULL);
+
+		if (child < 0) {
+			warn(_("fork failed"));
+			fail();
+		}
+		if (child)
+			dooutput();
+		else
+			doshell();
+	} else {
+		sa.sa_handler = resize;
+		sigaction(SIGWINCH, &sa, NULL);
+	}
+	doinput();
+
+	return EXIT_SUCCESS;
 }
-- 
2.2.1

[PATCH 14/14] script: add struct script_control and remove global variables

[Sami Kerola]

Fix also couple indentation issues.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 term-utils/script.c | 489 ++++++++++++++++++++++++++--------------------------
 1 file changed, 247 insertions(+), 242 deletions(-)

diff --git a/term-utils/script.c b/term-utils/script.c
index 0eaa73f..99c7fc8 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -12,8 +12,8 @@
  *    documentation and/or other materials provided with the distribution.
  * 3. All advertising materials mentioning features or use of this software
  *    must display the following acknowledgement:
- *	This product includes software developed by the University of
- *	California, Berkeley and its contributors.
+ *      This product includes software developed by the University of
+ *      California, Berkeley and its contributors.
  * 4. Neither the name of the University nor the names of its contributors
  *    may be used to endorse or promote products derived from this software
  *    without specific prior written permission.
@@ -80,42 +80,41 @@
 
 #define DEFAULT_OUTPUT "typescript"
 
-char	*shell;
-FILE	*fscript;
-FILE	*timingfd;
-int	master = -1;
-int	slave;
-pid_t	child;
-pid_t	subchild;
-int	childstatus;
-char	*fname;
-
-struct	termios tt;
-struct	winsize win;
-int	lb;
-int	l;
+struct script_control {
+	char *shell;		/* shell to be executed */
+	char *cflg;		/* command to be executed */
+	char *fname;		/* output file path */
+	FILE *typescriptfp;	/* output file pointer */
+	FILE *timingfp;		/* timing file pointer */
+	int master;		/* pseudoterminal master file descriptor */
+	int slave;		/* pseudoterminal slave file descriptor */
+	pid_t child;		/* child pid */
+	pid_t subchild;		/* subchild pid */
+	int childstatus;	/* child process exit value */
+	struct termios tt;	/* slave terminal runtime attributes */
+	struct winsize win;	/* terminal window size */
 #if !HAVE_LIBUTIL || !HAVE_PTY_H
-char	line[] = "/dev/ptyXX";
+	char line *;		/* terminal line */
 #endif
-int	aflg = 0;
-char	*cflg = NULL;
-int	eflg = 0;
-int	fflg = 0;
-int	qflg = 0;
-int	tflg = 0;
-int	forceflg = 0;
-int	isterm;
-
-sigset_t block_mask, unblock_mask;
-
-int die;
-int resized;
+	unsigned int
+	 aflg:1,		/* append output */
+	 eflg:1,		/* return child exit value */
+	 fflg:1,		/* flush after each write */
+	 qflg:1,		/* suppress most output */
+	 tflg:1,		/* include timing file */
+	 forceflg:1,		/* write output to links */
+	 isterm:1,		/* is child process running as terminal */
+	 resized:1,		/* has terminal been resized */
+	 die:1;			/* terminate program */
+	sigset_t block_mask;	/* signals that are blocked */
+	sigset_t unblock_mask;	/* original signal mask */
+};
+struct script_control *gctl;	/* global control structure, used in signal handlers */
 
 static void __attribute__((__noreturn__)) usage(FILE *out)
 {
 	fputs(USAGE_HEADER, out);
-	fprintf(out,
-	      _(" %s [options] [file]\n"), program_invocation_short_name);
+	fprintf(out, _(" %s [options] [file]\n"), program_invocation_short_name);
 
 	fputs(USAGE_OPTIONS, out);
 	fputs(_(" -a, --append            append the output\n"
@@ -132,17 +131,17 @@ static void __attribute__((__noreturn__)) usage(FILE *out)
 	exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
 }
 
-static void die_if_link(char *fn)
+static void die_if_link(const struct script_control *ctl)
 {
 	struct stat s;
 
-	if (forceflg)
+	if (ctl->forceflg)
 		return;
-	if (lstat(fn, &s) == 0 && (S_ISLNK(s.st_mode) || s.st_nlink > 1))
+	if (lstat(ctl->fname, &s) == 0 && (S_ISLNK(s.st_mode) || s.st_nlink > 1))
 		errx(EXIT_FAILURE,
 		     _("output file `%s' is a link\n"
 		       "Use --force if you really want to use it.\n"
-		       "Program not started."), fn);
+		       "Program not started."), ctl->fname);
 }
 
 /*
@@ -154,69 +153,67 @@ static void my_strftime(char *buf, size_t len, const char *fmt, const struct tm
 	strftime(buf, len, fmt, tm);
 }
 
-static void __attribute__((__noreturn__)) done(void)
+static void __attribute__((__noreturn__)) done(struct script_control *ctl)
 {
 	time_t tvec;
 
-	if (subchild) {
+	if (ctl->subchild) {
 		/* output process */
-		if (fscript) {
-			if (!qflg) {
+		if (ctl->typescriptfp) {
+			if (!ctl->qflg) {
 				char buf[BUFSIZ];
 				tvec = time((time_t *)NULL);
 				my_strftime(buf, sizeof buf, "%c\n", localtime(&tvec));
-				fprintf(fscript, _("\nScript done on %s"), buf);
+				fprintf(ctl->typescriptfp, _("\nScript done on %s"), buf);
 			}
-			if (close_stream(fscript) != 0)
+			if (close_stream(ctl->typescriptfp) != 0)
 				errx(EXIT_FAILURE, _("write error"));
-			fscript = NULL;
+			ctl->typescriptfp = NULL;
 		}
-		if (timingfd && close_stream(timingfd) != 0)
+		if (ctl->timingfp && close_stream(ctl->timingfp) != 0)
 			errx(EXIT_FAILURE, _("write error"));
-		timingfd = NULL;
+		ctl->timingfp = NULL;
 
-		close(master);
-		master = -1;
+		close(ctl->master);
+		ctl->master = -1;
 	} else {
 		/* input process */
-		if (isterm)
-			tcsetattr(STDIN_FILENO, TCSADRAIN, &tt);
-		if (!qflg)
-			printf(_("Script done, file is %s\n"), fname);
+		if (ctl->isterm)
+			tcsetattr(STDIN_FILENO, TCSADRAIN, &ctl->tt);
+		if (!ctl->qflg)
+			printf(_("Script done, file is %s\n"), ctl->fname);
 #ifdef HAVE_LIBUTEMPTER
-		if (master >= 0)
-			utempter_remove_record(master);
+		if (ctl->master >= 0)
+			utempter_remove_record(ctl->master);
 #endif
-		kill(child, SIGTERM);	/* make sure we don't create orphans */
+		kill(ctl->child, SIGTERM);	/* make sure we don't create orphans */
 	}
 
-	if(eflg) {
-		if (WIFSIGNALED(childstatus))
-			exit(WTERMSIG(childstatus) + 0x80);
+	if (ctl->eflg) {
+		if (WIFSIGNALED(ctl->childstatus))
+			exit(WTERMSIG(ctl->childstatus) + 0x80);
 		else
-			exit(WEXITSTATUS(childstatus));
+			exit(WEXITSTATUS(ctl->childstatus));
 	}
 	exit(EXIT_SUCCESS);
 }
 
-static void
-fail(void) {
-
+static void fail(struct script_control *ctl)
+{
 	kill(0, SIGTERM);
-	done();
+	done(ctl);
 }
 
-
-static void wait_for_empty_fd(int fd)
+static void wait_for_empty_fd(struct script_control *ctl, int fd)
 {
 	struct pollfd fds[] = {
-		{ .fd = fd, .events = POLLIN }
+		{.fd = fd, .events = POLLIN}
 	};
 
-	while (die == 0 && poll(fds, 1, 100) == 1);
+	while (ctl->die == 0 && poll(fds, 1, 100) == 1) ;
 }
 
-static void finish(int wait)
+static void finish(struct script_control *ctl, int wait)
 {
 	int status;
 	pid_t pid;
@@ -224,15 +221,15 @@ static void finish(int wait)
 	int options = wait ? 0 : WNOHANG;
 
 	while ((pid = wait3(&status, options, 0)) > 0)
-		if (pid == child) {
-			childstatus = status;
-			die = 1;
+		if (pid == ctl->child) {
+			ctl->childstatus = status;
+			ctl->die = 1;
 		}
 
 	errno = errsv;
 }
 
-static void doinput(void)
+static void doinput(struct script_control *ctl)
 {
 	int errsv = 0;
 	ssize_t cc = 0;
@@ -240,41 +237,40 @@ static void doinput(void)
 	fd_set readfds;
 
 	/* close things irrelevant for this process */
-	if (fscript)
-		fclose(fscript);
-	if (timingfd)
-		fclose(timingfd);
-	fscript = timingfd = NULL;
+	if (ctl->typescriptfp)
+		fclose(ctl->typescriptfp);
+	if (ctl->timingfp)
+		fclose(ctl->timingfp);
+	ctl->typescriptfp = ctl->timingfp = NULL;
 
 	FD_ZERO(&readfds);
 
 	/* block SIGCHLD */
-	sigprocmask(SIG_SETMASK, &block_mask, &unblock_mask);
+	sigprocmask(SIG_SETMASK, &ctl->block_mask, &ctl->unblock_mask);
 
-	while (die == 0) {
+	while (!ctl->die) {
 		FD_SET(STDIN_FILENO, &readfds);
 
 		errno = 0;
 		/* wait for input or signal (including SIGCHLD) */
 		if ((cc = pselect(STDIN_FILENO + 1, &readfds, NULL, NULL, NULL,
-			&unblock_mask)) > 0) {
+				  &ctl->unblock_mask)) > 0) {
 
 			if ((cc = read(STDIN_FILENO, ibuf, BUFSIZ)) > 0) {
-				if (write_all(master, ibuf, cc)) {
-					warn (_("write failed"));
-					fail();
+				if (write_all(ctl->master, ibuf, cc)) {
+					warn(_("write failed"));
+					fail(ctl);
 				}
 			}
 		}
 
-		if (cc < 0 && errno == EINTR && resized)
-		{
+		if (cc < 0 && errno == EINTR && ctl->resized) {
 			/* transmit window change information to the child */
-			if (isterm) {
-				ioctl(STDIN_FILENO, TIOCGWINSZ, (char *)&win);
-				ioctl(slave, TIOCSWINSZ, (char *)&win);
+			if (ctl->isterm) {
+				ioctl(STDIN_FILENO, TIOCGWINSZ, (char *)&ctl->win);
+				ioctl(ctl->slave, TIOCSWINSZ, (char *)&ctl->win);
 			}
-			resized = 0;
+			ctl->resized = 0;
 
 		} else if (cc <= 0 && errno != EINTR) {
 			errsv = errno;
@@ -283,13 +279,13 @@ static void doinput(void)
 	}
 
 	/* unblock SIGCHLD */
-	sigprocmask(SIG_SETMASK, &unblock_mask, NULL);
+	sigprocmask(SIG_SETMASK, &ctl->unblock_mask, NULL);
 
 	/* To be sure that we don't miss any data */
-	wait_for_empty_fd(slave);
-	wait_for_empty_fd(master);
+	wait_for_empty_fd(ctl, ctl->slave);
+	wait_for_empty_fd(ctl, ctl->master);
 
-	if (die == 0 && cc == 0 && errsv == 0) {
+	if (ctl->die == 0 && cc == 0 && errsv == 0) {
 		/*
 		 * Forward EOF from stdin (detected by read() above) to slave
 		 * (shell) to correctly terminate the session. It seems we have
@@ -301,153 +297,155 @@ static void doinput(void)
 		 */
 		int c = DEF_EOF;
 
-		if (write_all(master, &c, 1)) {
-			warn (_("write failed"));
-			fail();
+		if (write_all(ctl->master, &c, 1)) {
+			warn(_("write failed"));
+			fail(ctl);
 		}
 
 		/* wait for "exit" message from shell before we print "Script
 		 * done" in done() */
-		wait_for_empty_fd(master);
+		wait_for_empty_fd(ctl, ctl->master);
 	}
 
-	if (!die)
-		finish(1);	/* wait for children */
-	done();
+	if (!ctl->die)
+		finish(ctl, 1);	/* wait for children */
+	done(ctl);
 }
 
-static void sig_finish(int dummy __attribute__ ((__unused__)))
+static void sig_finish(int dummy __attribute__((__unused__)))
 {
-	finish(0);
+	finish(gctl, 0);
 }
 
-static void resize(int dummy __attribute__ ((__unused__)))
+static void resize(int dummy __attribute__((__unused__)))
 {
-	resized = 1;
+	gctl->resized = 1;
 }
 
-static void dooutput(void)
+static void dooutput(struct script_control *ctl)
 {
 	ssize_t cc;
 	char obuf[BUFSIZ];
 	struct timeval tv;
-	double oldtime=time(NULL), newtime;
+	double oldtime = time(NULL), newtime;
 	int errsv = 0;
 	fd_set readfds;
 
 	close(STDIN_FILENO);
 #ifdef HAVE_LIBUTIL
-	close(slave);
+	close(ctl->slave);
 #endif
-	if (tflg && !timingfd)
-		timingfd = fdopen(STDERR_FILENO, "w");
+	if (ctl->tflg && !ctl->timingfp)
+		ctl->timingfp = fdopen(STDERR_FILENO, "w");
 
-	if (!qflg) {
+	if (!ctl->qflg) {
 		time_t tvec = time((time_t *)NULL);
 		my_strftime(obuf, sizeof obuf, "%c\n", localtime(&tvec));
-		fprintf(fscript, _("Script started on %s"), obuf);
+		fprintf(ctl->typescriptfp, _("Script started on %s"), obuf);
 	}
 
 	FD_ZERO(&readfds);
 
 	do {
-		if (die || errsv == EINTR) {
-			struct pollfd fds[] = {{ .fd = master, .events = POLLIN }};
+		if (ctl->die || errsv == EINTR) {
+			struct pollfd fds[] = {
+				{.fd = ctl->master, .events = POLLIN}
+			};
 			if (poll(fds, 1, 50) <= 0)
 				break;
 		}
 
 		/* block SIGCHLD */
-		sigprocmask(SIG_SETMASK, &block_mask, &unblock_mask);
+		sigprocmask(SIG_SETMASK, &ctl->block_mask, &ctl->unblock_mask);
 
-		FD_SET(master, &readfds);
+		FD_SET(ctl->master, &readfds);
 		errno = 0;
 
 		/* wait for input or signal (including SIGCHLD) */
-		if ((cc = pselect(master+1, &readfds, NULL, NULL, NULL,
-			&unblock_mask)) > 0) {
+		if ((cc = pselect(ctl->master + 1, &readfds, NULL, NULL, NULL,
+				  &ctl->unblock_mask)) > 0) {
 
-			cc = read(master, obuf, sizeof (obuf));
+			cc = read(ctl->master, obuf, sizeof(obuf));
 		}
 		errsv = errno;
 
 		/* unblock SIGCHLD */
-		sigprocmask(SIG_SETMASK, &unblock_mask, NULL);
+		sigprocmask(SIG_SETMASK, &ctl->unblock_mask, NULL);
 
-		if (tflg)
+		if (ctl->tflg)
 			gettimeofday(&tv, NULL);
 
 		if (errsv == EINTR && cc <= 0)
 			continue;	/* try it again */
 		if (cc <= 0)
 			break;
-		if (tflg && timingfd) {
-			newtime = tv.tv_sec + (double) tv.tv_usec / 1000000;
-			fprintf(timingfd, "%f %zd\n", newtime - oldtime, cc);
+		if (ctl->tflg && ctl->timingfp) {
+			newtime = tv.tv_sec + (double)tv.tv_usec / 1000000;
+			fprintf(ctl->timingfp, "%f %zd\n", newtime - oldtime, cc);
 			oldtime = newtime;
 		}
-		if (fwrite_all(obuf, 1, cc, fscript)) {
-			warn (_("cannot write script file"));
-			fail();
+		if (fwrite_all(obuf, 1, cc, ctl->typescriptfp)) {
+			warn(_("cannot write script file"));
+			fail(ctl);
 		}
-		if (fflg) {
-			fflush(fscript);
-			if (tflg && timingfd)
-				fflush(timingfd);
+		if (ctl->fflg) {
+			fflush(ctl->typescriptfp);
+			if (ctl->tflg && ctl->timingfp)
+				fflush(ctl->timingfp);
 		}
 		if (write_all(STDOUT_FILENO, obuf, cc)) {
-			warn (_("write failed"));
-			fail();
+			warn(_("write failed"));
+			fail(ctl);
 		}
-	} while(1);
+	} while (1);
 
-	done();
+	done(ctl);
 }
 
-static void getslave(void)
+static void getslave(struct script_control *ctl)
 {
 #ifndef HAVE_LIBUTIL
 	line[strlen("/dev/")] = 't';
-	slave = open(line, O_RDWR);
-	if (slave < 0) {
-		warn(_("cannot open %s"), line);
-		fail();
+	ctl->slave = open(ctl->line, O_RDWR);
+	if (ctl->slave < 0) {
+		warn(_("cannot open %s"), ctl->line);
+		fail(ctl);
 	}
-	if (isterm) {
-		tcsetattr(slave, TCSANOW, &tt);
-		ioctl(slave, TIOCSWINSZ, (char *)&win);
+	if (ctl->isterm) {
+		tcsetattr(ctl->slave, TCSANOW, &ctl->tt);
+		ioctl(ctl->slave, TIOCSWINSZ, (char *)&ctl->win);
 	}
 #endif
 	setsid();
-	ioctl(slave, TIOCSCTTY, 0);
+	ioctl(ctl->slave, TIOCSCTTY, 0);
 }
 
-static void doshell(void)
+static void doshell(struct script_control *ctl)
 {
 	char *shname;
 
-	getslave();
+	getslave(ctl);
 
 	/* close things irrelevant for this process */
-	close(master);
-	if (fscript)
-		fclose(fscript);
-	if (timingfd)
-		fclose(timingfd);
-	fscript = timingfd = NULL;
+	close(ctl->master);
+	if (ctl->typescriptfp)
+		fclose(ctl->typescriptfp);
+	if (ctl->timingfp)
+		fclose(ctl->timingfp);
+	ctl->typescriptfp = ctl->timingfp = NULL;
 
-	dup2(slave, STDIN_FILENO);
-	dup2(slave, STDOUT_FILENO);
-	dup2(slave, STDERR_FILENO);
-	close(slave);
+	dup2(ctl->slave, STDIN_FILENO);
+	dup2(ctl->slave, STDOUT_FILENO);
+	dup2(ctl->slave, STDERR_FILENO);
+	close(ctl->slave);
 
-	master = -1;
+	ctl->master = -1;
 
-	shname = strrchr(shell, '/');
+	shname = strrchr(ctl->shell, '/');
 	if (shname)
 		shname++;
 	else
-		shname = shell;
+		shname = ctl->shell;
 
 	/*
 	 * When invoked from within /etc/csh.login, script spawns a csh shell
@@ -459,93 +457,92 @@ static void doshell(void)
 	 */
 	signal(SIGTERM, SIG_DFL);
 
-	if (access(shell, X_OK) == 0) {
-		if (cflg)
-			execl(shell, shname, "-c", cflg, NULL);
+	if (access(ctl->shell, X_OK) == 0) {
+		if (ctl->cflg)
+			execl(ctl->shell, shname, "-c", ctl->cflg, NULL);
 		else
-			execl(shell, shname, "-i", NULL);
+			execl(ctl->shell, shname, "-i", NULL);
 	} else {
-		if (cflg)
-			execlp(shname, "-c", cflg, NULL);
+		if (ctl->cflg)
+			execlp(shname, "-c", ctl->cflg, NULL);
 		else
 			execlp(shname, "-i", NULL);
 	}
-	warn(_("failed to execute %s"), shell);
-	fail();
+	warn(_("failed to execute %s"), ctl->shell);
+	fail(ctl);
 }
 
-static void fixtty(void)
+static void fixtty(struct script_control *ctl)
 {
 	struct termios rtt;
 
-	if (!isterm)
+	if (!ctl->isterm)
 		return;
 
-	rtt = tt;
+	rtt = ctl->tt;
 	cfmakeraw(&rtt);
 	rtt.c_lflag &= ~ECHO;
 	tcsetattr(STDIN_FILENO, TCSANOW, &rtt);
 }
 
-static void getmaster(void)
+static void getmaster(struct script_control *ctl)
 {
 #if defined(HAVE_LIBUTIL) && defined(HAVE_PTY_H)
 	int rc;
 
-	isterm = isatty(STDIN_FILENO);
+	ctl->isterm = isatty(STDIN_FILENO);
 
-	if (isterm) {
-		if (tcgetattr(STDIN_FILENO, &tt) != 0)
+	if (ctl->isterm) {
+		if (tcgetattr(STDIN_FILENO, &ctl->tt) != 0)
 			err(EXIT_FAILURE, _("failed to get terminal attributes"));
-		ioctl(STDIN_FILENO, TIOCGWINSZ, (char *) &win);
-		rc = openpty(&master, &slave, NULL, &tt, &win);
+		ioctl(STDIN_FILENO, TIOCGWINSZ, (char *)&ctl->win);
+		rc = openpty(&ctl->master, &ctl->slave, NULL, &ctl->tt, &ctl->win);
 	} else
-		rc = openpty(&master, &slave, NULL, NULL, NULL);
+		rc = openpty(&ctl->master, &ctl->slave, NULL, NULL, NULL);
 
 	if (rc < 0) {
 		warn(_("openpty failed"));
-		fail();
+		fail(ctl);
 	}
 #else
 	char *pty, *bank, *cp;
 	struct stat stb;
 
-	isterm = isatty(STDIN_FILENO);
+	ctl->isterm = isatty(STDIN_FILENO);
 
-	pty = &line[strlen("/dev/ptyp")];
+	pty = &ctl->line[strlen("/dev/ptyp")];
 	for (bank = "pqrs"; *bank; bank++) {
-		line[strlen("/dev/pty")] = *bank;
+		ctl->line[strlen("/dev/pty")] = *bank;
 		*pty = '0';
-		if (stat(line, &stb) < 0)
+		if (stat(ctl->line, &stb) < 0)
 			break;
 		for (cp = "0123456789abcdef"; *cp; cp++) {
 			*pty = *cp;
-			master = open(line, O_RDWR);
-			if (master >= 0) {
-				char *tp = &line[strlen("/dev/")];
+			ctl->master = open(ctl->line, O_RDWR);
+			if (ctl->master >= 0) {
+				char *tp = &ctl->line[strlen("/dev/")];
 				int ok;
 
 				/* verify slave side is usable */
 				*tp = 't';
-				ok = access(line, R_OK|W_OK) == 0;
+				ok = access(ctl->line, R_OK | W_OK) == 0;
 				*tp = 'p';
 				if (ok) {
-					if (isterm) {
-						tcgetattr(STDIN_FILENO, &tt);
-						ioctl(STDIN_FILENO, TIOCGWINSZ,
-								(char *)&win);
+					if (ctl->isterm) {
+						tcgetattr(STDIN_FILENO, &ctl->tt);
+						ioctl(STDIN_FILENO, TIOCGWINSZ, (char *)&ctl->win);
 					}
 					return;
 				}
-				close(master);
-				master = -1;
+				close(ctl->master);
+				ctl->master = -1;
 			}
 		}
 	}
-	master = -1;
+	ctl->master = -1;
 	warn(_("out of pty's"));
-	fail();
-#endif /* not HAVE_LIBUTIL */
+	fail(ctl);
+#endif				/* not HAVE_LIBUTIL */
 }
 
 /*
@@ -558,22 +555,29 @@ static void getmaster(void)
  */
 int main(int argc, char **argv)
 {
+	struct script_control ctl = {
+#if !HAVE_LIBUTIL || !HAVE_PTY_H
+		.line = "/dev/ptyXX",
+#endif
+		.master = -1,
+		0
+	};
 	struct sigaction sa;
 	int ch;
 
 	enum { FORCE_OPTION = CHAR_MAX + 1 };
 
 	static const struct option longopts[] = {
-		{ "append",	no_argument,	   NULL, 'a' },
-		{ "command",	required_argument, NULL, 'c' },
-		{ "return",	no_argument,	   NULL, 'e' },
-		{ "flush",	no_argument,	   NULL, 'f' },
-		{ "force",	no_argument,	   NULL, FORCE_OPTION, },
-		{ "quiet",	no_argument,	   NULL, 'q' },
-		{ "timing",	optional_argument, NULL, 't' },
-		{ "version",	no_argument,	   NULL, 'V' },
-		{ "help",	no_argument,	   NULL, 'h' },
-		{ NULL,		0, NULL, 0 }
+		{"append", no_argument, NULL, 'a'},
+		{"command", required_argument, NULL, 'c'},
+		{"return", no_argument, NULL, 'e'},
+		{"flush", no_argument, NULL, 'f'},
+		{"force", no_argument, NULL, FORCE_OPTION,},
+		{"quiet", no_argument, NULL, 'q'},
+		{"timing", optional_argument, NULL, 't'},
+		{"version", no_argument, NULL, 'V'},
+		{"help", no_argument, NULL, 'h'},
+		{NULL, 0, NULL, 0}
 	};
 
 	setlocale(LC_ALL, "");
@@ -581,31 +585,32 @@ int main(int argc, char **argv)
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
 	atexit(close_stdout);
+	gctl = &ctl;
 
 	while ((ch = getopt_long(argc, argv, "ac:efqt::Vh", longopts, NULL)) != -1)
-		switch(ch) {
+		switch (ch) {
 		case 'a':
-			aflg = 1;
+			ctl.aflg = 1;
 			break;
 		case 'c':
-			cflg = optarg;
+			ctl.cflg = optarg;
 			break;
 		case 'e':
-			eflg = 1;
+			ctl.eflg = 1;
 			break;
 		case 'f':
-			fflg = 1;
+			ctl.fflg = 1;
 			break;
 		case FORCE_OPTION:
-			forceflg = 1;
+			ctl.forceflg = 1;
 			break;
 		case 'q':
-			qflg = 1;
+			ctl.qflg = 1;
 			break;
 		case 't':
-			if (optarg && !(timingfd = fopen(optarg, "w")))
+			if (optarg && !(ctl.timingfp = fopen(optarg, "w")))
 				err(EXIT_FAILURE, _("cannot open %s"), optarg);
-			tflg = 1;
+			ctl.tflg = 1;
 			break;
 		case 'V':
 			printf(UTIL_LINUX_VERSION);
@@ -622,25 +627,25 @@ int main(int argc, char **argv)
 	argv += optind;
 
 	if (argc > 0)
-		fname = argv[0];
+		ctl.fname = argv[0];
 	else {
-		fname = DEFAULT_OUTPUT;
-		die_if_link(fname);
+		ctl.fname = DEFAULT_OUTPUT;
+		die_if_link(&ctl);
 	}
 
-	if ((fscript = fopen(fname, aflg ? "a" : "w")) == NULL) {
-		warn(_("cannot open %s"), fname);
-		fail();
+	if ((ctl.typescriptfp = fopen(ctl.fname, ctl.aflg ? "a" : "w")) == NULL) {
+		warn(_("cannot open %s"), ctl.fname);
+		fail(&ctl);
 	}
 
-	shell = getenv("SHELL");
-	if (shell == NULL)
-		shell = _PATH_BSHELL;
+	ctl.shell = getenv("SHELL");
+	if (ctl.shell == NULL)
+		ctl.shell = _PATH_BSHELL;
 
-	getmaster();
-	if (!qflg)
-		printf(_("Script started, file is %s\n"), fname);
-	fixtty();
+	getmaster(&ctl);
+	if (!ctl.qflg)
+		printf(_("Script started, file is %s\n"), ctl.fname);
+	fixtty(&ctl);
 
 #ifdef HAVE_LIBUTEMPTER
 	utempter_add_record(master, NULL);
@@ -652,36 +657,36 @@ int main(int argc, char **argv)
 	sigaction(SIGCHLD, &sa, NULL);
 
 	/* init mask for SIGCHLD */
-	sigprocmask(SIG_SETMASK, NULL, &block_mask);
-	sigaddset(&block_mask, SIGCHLD);
+	sigprocmask(SIG_SETMASK, NULL, &ctl.block_mask);
+	sigaddset(&ctl.block_mask, SIGCHLD);
 
-	sigprocmask(SIG_SETMASK, &block_mask, &unblock_mask);
-	child = fork();
-	sigprocmask(SIG_SETMASK, &unblock_mask, NULL);
+	sigprocmask(SIG_SETMASK, &ctl.block_mask, &ctl.unblock_mask);
+	ctl.child = fork();
+	sigprocmask(SIG_SETMASK, &ctl.unblock_mask, NULL);
 
-	if (child < 0) {
+	if (ctl.child < 0) {
 		warn(_("fork failed"));
-		fail();
+		fail(&ctl);
 	}
-	if (child == 0) {
+	if (ctl.child == 0) {
 
-		sigprocmask(SIG_SETMASK, &block_mask, NULL);
-		subchild = child = fork();
-		sigprocmask(SIG_SETMASK, &unblock_mask, NULL);
+		sigprocmask(SIG_SETMASK, &ctl.block_mask, NULL);
+		ctl.subchild = ctl.child = fork();
+		sigprocmask(SIG_SETMASK, &ctl.unblock_mask, NULL);
 
-		if (child < 0) {
+		if (ctl.child < 0) {
 			warn(_("fork failed"));
-			fail();
+			fail(&ctl);
 		}
-		if (child)
-			dooutput();
+		if (ctl.child)
+			dooutput(&ctl);
 		else
-			doshell();
+			doshell(&ctl);
 	} else {
 		sa.sa_handler = resize;
 		sigaction(SIGWINCH, &sa, NULL);
 	}
-	doinput();
+	doinput(&ctl);
 
 	return EXIT_SUCCESS;
 }
-- 
2.2.1

Re: [PATCH 14/14] script: add struct script_control and remove global variables

[Sami Kerola]

On Mon, 22 Dec 2014, Sami Kerola wrote:
>  	line[strlen("/dev/")] = 't';

The above is incorrect.  Following fix can be found from my github 
'script' branch.

@@ -423,7 +423,7 @@ static void dooutput(struct script_control *ctl)
 static void getslave(struct script_control *ctl)
 {
 #ifndef HAVE_LIBUTIL
-       line[strlen("/dev/")] = 't';
+       ctl->line[strlen("/dev/")] = 't';

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

Re: [PATCH 00/14] pull: chsh, newgrp, and script changes

[Karel Zak]

 Hi,

On Mon, Dec 22, 2014 at 11:47:03AM +0000, Sami Kerola wrote:
>   chsh: remove function prototypes
>   chfn, chsh: share illegal_passwd_chars() function
>   chsh: use getline() to support arbitrarily long lines
>   chsh: set few variables read-only and rename one of them
>   chsh: allow user to set shell to /bin/sh if none is set
>   chsh: clean up parse_argv()
>   chsh: rewrite function interacting with user to get path to new shell
>   chsh: simplify check_shell()
>   chsh: fail get_shell_list() check when /etc/shells cannot be opened
>   newgrp: simplify if else clauses
>   newgrp: move shell determination closer where it is used
>   newgrp: set function arguments read-only when possible

I have merged 'chsh' and 'newgrp' branches from github.

>   script: remove function prototypes
>   script: add struct script_control and remove global variables

I'm going to merge this together with another changes from 'script' branch.

 Thanks!

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com