[PATCH 1/4] script: option --force added

[PATCH 1/4] script: option --force added

[Sami Kerola]

The --force will allow default output destination, e.g.
typescript file, to be hard or symbolic link.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 term-utils/script.1 |    3 +++
 term-utils/script.c |   21 ++++++++++++++-------
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/term-utils/script.1 b/term-utils/script.1
index 725bdb2..f618ec3 100644
--- a/term-utils/script.1
+++ b/term-utils/script.1
@@ -84,6 +84,9 @@ termination on signal termination exit code is 128+n.
 Flush output after each write. This is nice for telecooperation:
 One person does `mkfifo foo; script -f foo' and another can
 supervise real-time what is being done using `cat foo'.
+.It Fl Fl force
+Allow default output destination, e.g. typescript file, to be
+hard or symbolic link. The command will follow symbolic link.
 .It Fl q, Fl Fl quiet
 Be quiet.
 .It Fl t, Fl Fl timing[=FILE]
diff --git a/term-utils/script.c b/term-utils/script.c
index 603d392..1753849 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -101,6 +101,7 @@ int	eflg = 0;
 int	fflg = 0;
 int	qflg = 0;
 int	tflg = 0;
+int	forceflg = 0;
 
 int die;
 int resized;
@@ -109,14 +110,13 @@ 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))
-	        /* FIXME: there is no [options] to allow/force this to happen.  */
 		errx(EXIT_FAILURE,
-			_("Warning: `%s' is a link.\n"
-			  "Use `%s [options] %s' if you really "
-			  "want to use it.\n"
-			  "Program not started.\n"),
-			fn, program_invocation_short_name, fn);
+		     _("error: 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__))
@@ -132,6 +132,7 @@ usage(FILE *out)
 		" -c, --command COMMAND   run command rather than interactive shell\n"
 		" -r, --return            return exit code of the child process\n"
 		" -f, --flush             run flush after each write\n"
+		"     --force             use output file even it would be a link\n"
 		" -q, --quiet             be quiet\n"
 		" -t, --timing=FILE       output timing data to stderr, or to file\n"
 		" -V, --version           output version information and exit\n"
@@ -157,11 +158,14 @@ main(int argc, char **argv) {
 	int ch;
 	FILE *timingfd = stderr;
 
+	enum { FORCE_OPTION = CHAR_MAX + 1 };
+
 	static const struct option longopts[] = {
 		{ "append",	no_argument,	   0, 'a' },
 		{ "command",	required_argument, 0, 'c' },
 		{ "return",	no_argument,	   0, 'e' },
 		{ "flush",	no_argument,	   0, 'f' },
+		{ "force",	no_argument,	   0, FORCE_OPTION, },
 		{ "quiet",	no_argument,	   0, 'q' },
 		{ "timing",	optional_argument, 0, 't' },
 		{ "version",	no_argument,	   0, 'V' },
@@ -175,7 +179,7 @@ main(int argc, char **argv) {
 	textdomain(PACKAGE);
 
 	while ((ch = getopt_long(argc, argv, "ac:efqt::Vh", longopts, NULL)) != -1)
-		switch((char)ch) {
+		switch(ch) {
 		case 'a':
 			aflg++;
 			break;
@@ -188,6 +192,9 @@ main(int argc, char **argv) {
 		case 'f':
 			fflg++;
 			break;
+		case FORCE_OPTION:
+			forceflg = 1;
+			break;
 		case 'q':
 			qflg++;
 			break;
-- 
1.7.5.2

[PATCH 2/4] script: remove unnecessary void casting

[Sami Kerola]

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

diff --git a/term-utils/script.c b/term-utils/script.c
index 1753849..601a91d 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -289,7 +289,7 @@ doinput() {
 	register int cc;
 	char ibuf[BUFSIZ];
 
-	(void) fclose(fscript);
+	fclose(fscript);
 
 	while (die == 0) {
 		if ((cc = read(0, ibuf, BUFSIZ)) > 0) {
@@ -326,8 +326,8 @@ void
 resize(int dummy __attribute__ ((__unused__))) {
 	resized = 1;
 	/* transmit window change information to the child */
-	(void) ioctl(0, TIOCGWINSZ, (char *)&win);
-	(void) ioctl(slave, TIOCSWINSZ, (char *)&win);
+	ioctl(0, TIOCGWINSZ, (char *)&win);
+	ioctl(slave, TIOCSWINSZ, (char *)&win);
 }
 
 /*
@@ -350,9 +350,9 @@ dooutput(FILE *timingfd) {
 	ssize_t wrt;
 	ssize_t fwrt;
 
-	(void) close(0);
+	close(0);
 #ifdef HAVE_LIBUTIL
-	(void) close(slave);
+	close(slave);
 #endif
 	tvec = time((time_t *)NULL);
 	my_strftime(obuf, sizeof obuf, "%c\n", localtime(&tvec));
@@ -396,7 +396,7 @@ dooutput(FILE *timingfd) {
 			fail();
 		}
 		if (fflg)
-			(void) fflush(fscript);
+			fflush(fscript);
 	} while(1);
 
 	if (flgs)
@@ -413,18 +413,18 @@ doshell() {
 
 	t = open(_PATH_DEV_TTY, O_RDWR);
 	if (t >= 0) {
-		(void) ioctl(t, TIOCNOTTY, (char *)0);
-		(void) close(t);
+		ioctl(t, TIOCNOTTY, (char *)0);
+		close(t);
 	}
 #endif
 
 	getslave();
-	(void) close(master);
-	(void) fclose(fscript);
-	(void) dup2(slave, 0);
-	(void) dup2(slave, 1);
-	(void) dup2(slave, 2);
-	(void) close(slave);
+	close(master);
+	fclose(fscript);
+	dup2(slave, 0);
+	dup2(slave, 1);
+	dup2(slave, 2);
+	close(slave);
 
 	master = -1;
 
@@ -450,13 +450,13 @@ fixtty() {
 	rtt = tt;
 	cfmakeraw(&rtt);
 	rtt.c_lflag &= ~ECHO;
-	(void) tcsetattr(0, TCSANOW, &rtt);
+	tcsetattr(0, TCSANOW, &rtt);
 }
 
 void
 fail() {
 
-	(void) kill(0, SIGTERM);
+	kill(0, SIGTERM);
 	done();
 }
 
@@ -471,12 +471,12 @@ done() {
 			my_strftime(buf, sizeof buf, "%c\n", localtime(&tvec));
 			fprintf(fscript, _("\nScript done on %s"), buf);
 		}
-		(void) fclose(fscript);
-		(void) close(master);
+		fclose(fscript);
+		close(master);
 
 		master = -1;
 	} else {
-		(void) tcsetattr(0, TCSADRAIN, &tt);
+		tcsetattr(0, TCSADRAIN, &tt);
 		if (!qflg)
 			printf(_("Script done, file is %s\n"), fname);
 #ifdef HAVE_LIBUTEMPTER
@@ -497,8 +497,8 @@ done() {
 void
 getmaster() {
 #if HAVE_LIBUTIL && HAVE_PTY_H
-	(void) tcgetattr(0, &tt);
-	(void) ioctl(0, TIOCGWINSZ, (char *)&win);
+	tcgetattr(0, &tt);
+	ioctl(0, TIOCGWINSZ, (char *)&win);
 	if (openpty(&master, &slave, NULL, &tt, &win) < 0) {
 		warn(_("openpty failed"));
 		fail();
@@ -525,12 +525,12 @@ getmaster() {
 				ok = access(line, R_OK|W_OK) == 0;
 				*tp = 'p';
 				if (ok) {
-					(void) tcgetattr(0, &tt);
-				    	(void) ioctl(0, TIOCGWINSZ, 
+					tcgetattr(0, &tt);
+					ioctl(0, TIOCGWINSZ,
 						(char *)&win);
 					return;
 				}
-				(void) close(master);
+				close(master);
 				master = -1;
 			}
 		}
@@ -550,9 +550,9 @@ getslave() {
 		warn(_("open failed: %s"), line);
 		fail();
 	}
-	(void) tcsetattr(slave, TCSANOW, &tt);
-	(void) ioctl(slave, TIOCSWINSZ, (char *)&win);
+	tcsetattr(slave, TCSANOW, &tt);
+	ioctl(slave, TIOCSWINSZ, (char *)&win);
 #endif
-	(void) setsid();
-	(void) ioctl(slave, TIOCSCTTY, 0);
+	setsid();
+	ioctl(slave, TIOCSCTTY, 0);
 }
-- 
1.7.5.2

Re: [PATCH 2/4] script: remove unnecessary void casting

[Karel Zak]

On Tue, Jun 28, 2011 at 12:43:24PM +0200, Sami Kerola wrote:
>  term-utils/script.c |   56 +++++++++++++++++++++++++-------------------------
>  1 files changed, 28 insertions(+), 28 deletions(-)

 Applied, thanks.

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

[PATCH 3/4] script: include-what-you-use header check

[Sami Kerola]

The tool proposed;

script.c should add these lines:

script.c should remove these lines:
- #include <sys/file.h>  // lines 53-53
- #include <sys/types.h>  // lines 48-48
- #include "c.h"  // lines 61-61

and the change nearly did what the tool told. We should keep on
using c.h, not err.h. The config.h is not needed, it's added
automaticly.

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

diff --git a/term-utils/script.c b/term-utils/script.c
index 601a91d..39bc5ef 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -45,17 +45,20 @@
 #include <stdlib.h>
 #include <paths.h>
 #include <time.h>
-#include <sys/types.h>
 #include <sys/stat.h>
 #include <termios.h>
 #include <sys/ioctl.h>
 #include <sys/time.h>
-#include <sys/file.h>
 #include <signal.h>
 #include <errno.h>
 #include <string.h>
 #include <getopt.h>
 #include <unistd.h>
+#include <fcntl.h>
+#include <libintl.h>
+#include <limits.h>
+#include <locale.h>
+#include <stddef.h>
 
 #include "nls.h"
 #include "c.h"
-- 
1.7.5.2

Re: [PATCH 3/4] script: include-what-you-use header check

[Karel Zak]

On Tue, Jun 28, 2011 at 12:43:25PM +0200, Sami Kerola wrote:
>  term-utils/script.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)

 Applied, thanks.

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

[PATCH 4/4] script: remove magic constants and a type mismatch fix

[Sami Kerola]

The type mismatch; doinput: int -> ssize_t

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 term-utils/script.c |   68 ++++++++++++++++++++++++++------------------------
 1 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/term-utils/script.c b/term-utils/script.c
index 39bc5ef..fccc0c4 100644
--- a/term-utils/script.c
+++ b/term-utils/script.c
@@ -71,6 +71,8 @@
 #include <utempter.h>
 #endif
 
+#define DEFAULT_OUTPUT "typescript"
+
 void finish(int);
 void done(void);
 void fail(void);
@@ -164,16 +166,16 @@ main(int argc, char **argv) {
 	enum { FORCE_OPTION = CHAR_MAX + 1 };
 
 	static const struct option longopts[] = {
-		{ "append",	no_argument,	   0, 'a' },
-		{ "command",	required_argument, 0, 'c' },
-		{ "return",	no_argument,	   0, 'e' },
-		{ "flush",	no_argument,	   0, 'f' },
-		{ "force",	no_argument,	   0, FORCE_OPTION, },
-		{ "quiet",	no_argument,	   0, 'q' },
-		{ "timing",	optional_argument, 0, 't' },
-		{ "version",	no_argument,	   0, 'V' },
-		{ "help",	no_argument,	   0, 'h' },
-		{ NULL,		0, 0, 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, "");
@@ -184,28 +186,28 @@ main(int argc, char **argv) {
 	while ((ch = getopt_long(argc, argv, "ac:efqt::Vh", longopts, NULL)) != -1)
 		switch(ch) {
 		case 'a':
-			aflg++;
+			aflg = 1;
 			break;
 		case 'c':
 			cflg = optarg;
 			break;
 		case 'e':
-			eflg++;
+			eflg = 1;
 			break;
 		case 'f':
-			fflg++;
+			fflg = 1;
 			break;
 		case FORCE_OPTION:
 			forceflg = 1;
 			break;
 		case 'q':
-			qflg++;
+			qflg = 1;
 			break;
 		case 't':
 			if (optarg)
 				if ((timingfd = fopen(optarg, "w")) == NULL)
 					err(EXIT_FAILURE, _("cannot open timing file %s"), optarg);
-			tflg++;
+			tflg = 1;
 			break;
 		case 'V':
 			printf(_("%s from %s\n"), program_invocation_short_name,
@@ -225,7 +227,7 @@ main(int argc, char **argv) {
 	if (argc > 0)
 		fname = argv[0];
 	else {
-		fname = "typescript";
+		fname = DEFAULT_OUTPUT;
 		die_if_link(fname);
 	}
 	if ((fscript = fopen(fname, aflg ? "a" : "w")) == NULL) {
@@ -289,20 +291,20 @@ main(int argc, char **argv) {
 
 void
 doinput() {
-	register int cc;
+	register ssize_t cc;
 	char ibuf[BUFSIZ];
 
 	fclose(fscript);
 
 	while (die == 0) {
-		if ((cc = read(0, ibuf, BUFSIZ)) > 0) {
+		if ((cc = read(STDIN_FILENO, ibuf, BUFSIZ)) > 0) {
 			ssize_t wrt = write(master, ibuf, cc);
-			if (wrt == -1) {
+			if (wrt < 0) {
 				warn (_("write failed"));
 				fail();
 			}
 		}
-		else if (cc == -1 && errno == EINTR && resized)
+		else if (cc < 0 && errno == EINTR && resized)
 			resized = 0;
 		else
 			break;
@@ -329,7 +331,7 @@ void
 resize(int dummy __attribute__ ((__unused__))) {
 	resized = 1;
 	/* transmit window change information to the child */
-	ioctl(0, TIOCGWINSZ, (char *)&win);
+	ioctl(STDIN_FILENO, TIOCGWINSZ, (char *)&win);
 	ioctl(slave, TIOCSWINSZ, (char *)&win);
 }
 
@@ -353,7 +355,7 @@ dooutput(FILE *timingfd) {
 	ssize_t wrt;
 	ssize_t fwrt;
 
-	close(0);
+	close(STDIN_FILENO);
 #ifdef HAVE_LIBUTIL
 	close(slave);
 #endif
@@ -388,7 +390,7 @@ dooutput(FILE *timingfd) {
 			fprintf(timingfd, "%f %zd\n", newtime - oldtime, cc);
 			oldtime = newtime;
 		}
-		wrt = write(1, obuf, cc);
+		wrt = write(STDOUT_FILENO, obuf, cc);
 		if (wrt < 0) {
 			warn (_("write failed"));
 			fail();
@@ -424,9 +426,9 @@ doshell() {
 	getslave();
 	close(master);
 	fclose(fscript);
-	dup2(slave, 0);
-	dup2(slave, 1);
-	dup2(slave, 2);
+	dup2(slave, STDIN_FILENO);
+	dup2(slave, STDOUT_FILENO);
+	dup2(slave, STDERR_FILENO);
 	close(slave);
 
 	master = -1;
@@ -453,13 +455,13 @@ fixtty() {
 	rtt = tt;
 	cfmakeraw(&rtt);
 	rtt.c_lflag &= ~ECHO;
-	tcsetattr(0, TCSANOW, &rtt);
+	tcsetattr(STDIN_FILENO, TCSANOW, &rtt);
 }
 
 void
 fail() {
 
-	kill(0, SIGTERM);
+	kill(getpid(), SIGTERM);
 	done();
 }
 
@@ -479,7 +481,7 @@ done() {
 
 		master = -1;
 	} else {
-		tcsetattr(0, TCSADRAIN, &tt);
+		tcsetattr(STDIN_FILENO, TCSADRAIN, &tt);
 		if (!qflg)
 			printf(_("Script done, file is %s\n"), fname);
 #ifdef HAVE_LIBUTEMPTER
@@ -500,8 +502,8 @@ done() {
 void
 getmaster() {
 #if HAVE_LIBUTIL && HAVE_PTY_H
-	tcgetattr(0, &tt);
-	ioctl(0, TIOCGWINSZ, (char *)&win);
+	tcgetattr(STDIN_FILENO, &tt);
+	ioctl(STDIN_FILENO, TIOCGWINSZ, (char *)&win);
 	if (openpty(&master, &slave, NULL, &tt, &win) < 0) {
 		warn(_("openpty failed"));
 		fail();
@@ -528,8 +530,8 @@ getmaster() {
 				ok = access(line, R_OK|W_OK) == 0;
 				*tp = 'p';
 				if (ok) {
-					tcgetattr(0, &tt);
-					ioctl(0, TIOCGWINSZ,
+					tcgetattr(STDIN_FILENO, &tt);
+					ioctl(STDIN_FILENO, TIOCGWINSZ,
 						(char *)&win);
 					return;
 				}
-- 
1.7.5.2

Re: [PATCH 4/4] script: remove magic constants and a type mismatch fix

[Karel Zak]

On Tue, Jun 28, 2011 at 12:43:26PM +0200, Sami Kerola wrote:
>  term-utils/script.c |   68 ++++++++++++++++++++++++++------------------------
>  1 files changed, 35 insertions(+), 33 deletions(-)

 Applied.

>>  void
>  fail() {
>  
> -	kill(0, SIGTERM);
> +	kill(getpid(), SIGTERM);
>  	done();
>  }

 I think we want to use zero rather than getpid()...

    Karel

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

Re: [PATCH 1/4] script: option --force added

[Karel Zak]

On Tue, Jun 28, 2011 at 12:43:23PM +0200, Sami Kerola wrote:
> The --force will allow default output destination, e.g.
> typescript file, to be hard or symbolic link.

 Applied, thanks.

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