Re: bin/55531: [PATCH] script(1): Check if stdin is a terminal + other error handling

[Soumendra Ganguly <soumendra%tamu.edu@localhost>]

--- src/usr.bin/script/script.c	2020-08-02 02:06:56.406214345 -0500
+++ script.c	2020-08-02 04:46:28.843706209 -0500
@@ -155,17 +155,28 @@
 	if (pflg)
 		playback(fscript);

-	(void)tcgetattr(STDIN_FILENO, &tt);
-	(void)ioctl(STDIN_FILENO, TIOCGWINSZ, &win);
-	if (openpty(&master, &slave, NULL, &tt, &win) == -1)
-		err(1, "openpty");
+	isterm = isatty(STDIN_FILENO);
+	if (isterm) {
+		if (tcgetattr(STDIN_FILENO, &tt) == -1)
+			err(1, "tcgetattr");
+		if (ioctl(STDIN_FILENO, TIOCGWINSZ, &win) == -1)
+			err(1, "ioctl");
+		if (openpty(&master, &slave, NULL, &tt, &win) == -1)
+			err(1, "openpty");
+	} else {
+		if (openpty(&master, &slave, NULL, NULL, NULL) == -1)
+			err(1, "openpty");		
+	}

 	if (!quiet)
 		(void)printf("Script started, output file is %s\n", fname);
-	rtt = tt;
-	cfmakeraw(&rtt);
-	rtt.c_lflag &= ~ECHO;
-	(void)tcsetattr(STDIN_FILENO, TCSAFLUSH, &rtt);
+
+	if (isterm) {
+		rtt = tt;
+		cfmakeraw(&rtt);
+		rtt.c_lflag &= ~ECHO;
+		(void)tcsetattr(STDIN_FILENO, TCSAFLUSH, &rtt);
+	}

 	(void)signal(SIGCHLD, finish);
 	child = fork();
@@ -301,7 +312,8 @@
 		(void)fclose(fscript);
 		(void)close(master);
 	} else {
-		(void)tcsetattr(STDIN_FILENO, TCSAFLUSH, &tt);
+		if (isterm)
+			(void)tcsetattr(STDIN_FILENO, TCSAFLUSH, &tt);
 		if (!quiet)
 			(void)printf("Script done, output file is %s\n", fname);
 	}
@@ -365,13 +377,13 @@
 	if (!isterm)
 		return;

-	if (tcgetattr(STDOUT_FILENO, &tt) != 0)
+	if (tcgetattr(STDOUT_FILENO, &tt) == -1)
 		err(1, "tcgetattr");

 	traw = tt;
 	cfmakeraw(&traw);
 	traw.c_lflag |= ISIG;
-	if (tcsetattr(STDOUT_FILENO, TCSANOW, &traw) != 0)
+	if (tcsetattr(STDOUT_FILENO, TCSANOW, &traw) == -1)
 		err(1, "tcsetattr");
 }

@@ -396,8 +408,10 @@
 	time_t tclock;
 	int reg;

+	isterm = 0;
+
 	if (fstat(fileno(fp), &pst) == -1)
-		err(1, "fstat failed");	
+		err(1, "fstat failed");

 	reg = S_ISREG(pst.st_mode);

@@ -431,6 +445,7 @@
 			atexit(termreset);
 			break;
 		case 'e':
+			termreset();
 			if (!quiet)
 				(void)printf("\nScript done on %s",
 				    ctime(&tclock));


On 8/2/20, Soumendra Ganguly <soumendra%tamu.edu@localhost> wrote:
> Hypothesis: reintroduction of the extra termreset() makes it necessary
> to have "isterm = 0" at the beginning of "playback()" so that if a
> malformed recording file forces case 'e' before case 's', then
> tcsetattr will not be called by termreset(). In that case, an updated
> patch follows. Also, this patch removes an extra whitespace after
> err(1, "fstat failed").
>
> On 8/2/20, Soumendra Ganguly <soumendra%tamu.edu@localhost> wrote:
>> The following reply was made to PR bin/55531; it has been noted by GNATS.
>>
>> From: Soumendra Ganguly <soumendra%tamu.edu@localhost>
>> To: gnats-bugs%netbsd.org@localhost
>> Cc:
>> Subject: Re: bin/55531: [PATCH] script(1): Check if stdin is a terminal +
>>  other error handling
>> Date: Sun, 2 Aug 2020 03:44:03 -0500
>>
>>  This follows #55529
>>
>>  On 8/2/20, gnats-admin%netbsd.org@localhost <gnats-admin%netbsd.org@localhost> wrote:
>>  > Thank you very much for your problem report.
>>  > It has the internal identification `bin/55531'.
>>  > The individual assigned to look at your
>>  > report is: bin-bug-people.
>>  >
>>  >>Category:       bin
>>  >>Responsible:    bin-bug-people
>>  >>Synopsis:       [PATCH] script(1): Check if stdin is a terminal +
>> other
>>  >> error handling
>>  >>Arrival-Date:   Sun Aug 02 08:30:00 +0000 2020
>>  >
>>  >
>>
>>
>