Fix bug on closed stdout; improve BUG_PUTIOERR fix (re: 93e15a30)

Stéphane Chazelas reported:

> As noted in this austin-group-l discussion[*] (relevant to this
> issue):
>
>   $ ksh93u+m -c 'pwd; echo "$?" >&2; echo test; echo "$?" >&2' >&-
>   0
>   1
>   /home/chazelas
>
> when stdout is closed, pwd does claim it succeeds (by returning a
> 0 exit status), while echo doesn't (not really relevant to the
> problem here, only to show it doesn't affect all builtins), and
> the output that pwd failed to write earlier ends up being written
> on stderr here instead of stdout upon exit (presumably) because
> of that >&2 redirection.
>
> strace shows ksh93 attempting write(1, "/home/chazelas\n", 15) 6
> times (1, the last one, successful).
>
> It gets even weirder when redirecting to a file:
>
>   $ ksh93u+m -c 'pwd; echo "$?" >&2; echo test; echo "$?" > file' >&-
>   0
>   $ cat file
>   1
>   1
>   ome/chazelas

In my testing, the problem does not occur when closing stdout at
the start of the -c script itself (using redirect >&- or exec >&-);
it only occurs if stdout was closed before initialising the shell.

That made me suspect that the problem had to do with an
inconsistent file descriptor state in the shell. ksh uses internal
sh_open() and sh_close() functions, among others, to maintain that
state.

src/cmd/ksh93/sh/main.c: sh_main():
- If the shell is initialised with stdin, stdout or stderr closed,
  then make the shell's file descriptor state tables reflect that
  fact by calling sh_close() for the closed file descriptors.

This commit also improves the BUG_PUTIOERR fix from 93e15a30. Error
checking after sfsync() is not sufficient. For instance, on
FreeBSD, the following did not produce a non-zero exit status:
  ksh -c 'echo hi' >/dev/full
even though this did:
  ksh -c 'echo hi >/dev/full'
Reliable error checking requires not only checking the result of
every SFIO command that writes output, but also synching the buffer
at the end of the operation and checking the result of that.

src/cmd/ksh93/bltins/print.c:
- Make exitval variable global to allow functions called by
  b_print() to set a nonzero exit status.
- Check the result of all SFIO output commands that write output.
- b_print(): Always sfsync() at the end, except if the s (history)
  flag was given. This allows getting rid of the sfsync() call that
  required the workaround introduced in 846ad932.

[*] https://www.mail-archive.com/austin-group-l@opengroup.org/msg08056.html

Resolves: https://github.com/ksh93/ksh/issues/314

NEWS

@@ -3,6 +3,15 @@ For full details, see the git log at: https://github.com/ksh93/ksh
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
+2021-11-07:
+
+- Fixed a bug that could corrupt output if standard output is closed upon
+  initializing the shell.
+
+- Improved BUG_PUTIOERR fix (2020-05-14) with more error checking. On
+  systems with the "disk full" error testing device /dev/full, an
+  echo/print/printf to /dev/full now always yields a non-zero exit status.
+
 2021-09-13:
 
 - Fixed a bug introduced in 93u+ 2012-02-07 that caused the 'printf' builtin

src/cmd/ksh93/bltins/print.c

@@ -100,6 +100,7 @@ struct print
 };
 
 static char* 	nullarg[] = { 0, 0 };
+static int	exitval;
 
 #if !SHOPT_ECHOPRINT
    int    B_echo(int argc, char *argv[],Shbltin_t *context)
@@ -172,12 +173,13 @@ static int infof(Opt_t* op, Sfio_t* sp, const char* s, Optdisc_t* dp)
 int    b_print(int argc, char *argv[], Shbltin_t *context)
 {
 	register Sfio_t *outfile;
-	register int exitval=0,n, fd = 1;
+	register int n, fd = 1;
 	register Shell_t *shp = context->shp;
 	const char *options, *msg = e_file+4;
 	char *format = 0;
 	int sflag = 0, nflag=0, rflag=0, vflag=0;
 	Optdisc_t disc;
+	exitval = 0;
 	disc.version = OPT_VERSION;
 	disc.infof = infof;
 	opt_info.disc = &disc;
@@ -335,15 +337,7 @@ skip2:
 			sfprintf(outfile,"%!",&pdata);
 		} while(*pdata.nextarg && pdata.nextarg!=argv);
 		if(pdata.nextarg == nullarg && pdata.argsize>0)
-			sfwrite(outfile,stakptr(staktell()),pdata.argsize);
-		/*
-		 * -f flag skips adding newline at the end of output, which causes issues
-		 * with syncing history if -s and -f are used together. So don't sync now
-		 * if sflag was given. History is synced later with hist_flush() function.
-		 * https://github.com/att/ast/issues/425
-		 */
-		if(!sflag && sffileno(outfile)!=sffileno(sfstderr))
-			if (sfsync(outfile) < 0)
+			if(sfwrite(outfile,stakptr(staktell()),pdata.argsize) < 0)
 				exitval = 1;
 		sfpool(sfstderr,pool,SF_WRITE);
 		if (pdata.err)
@@ -353,9 +347,11 @@ skip2:
 	{
 		while(*argv)
 		{
-			fmtbase64(outfile,*argv++,vflag=='C');
+			if(fmtbase64(outfile,*argv++,vflag=='C') < 0)
+				exitval = 1;
 			if(!nflag)
-				sfputc(outfile,'\n');
+				if(sfputc(outfile,'\n') < 0)
+					exitval = 1;
 		}
 	}
 	else
@@ -367,16 +363,18 @@ skip2:
 				exitval = 1;
 		}
 		else if(sh_echolist(shp,outfile,rflag,argv) && !nflag)
-			sfputc(outfile,'\n');
+			if(sfputc(outfile,'\n') < 0)
+				exitval = 1;
 	}
 	if(sflag)
 	{
 		hist_flush(shp->gd->hist_ptr);
 		sh_offstate(SH_HISTORY);
 	}
-	else if(n&SF_SHARE)
+	else
 	{
-		sfset(outfile,SF_SHARE|SF_PUBLIC,1);
+		if(n&SF_SHARE)
+			sfset(outfile,SF_SHARE|SF_PUBLIC,1);
 		if (sfsync(outfile) < 0)
 			exitval = 1;
 	}
@@ -401,12 +399,15 @@ int sh_echolist(Shell_t *shp,Sfio_t *outfile, int raw, char *argv[])
 		if(!raw  && (n=fmtvecho(cp,&pdata))>=0)
 		{
 			if(n)
-				sfwrite(outfile,stakptr(staktell()),n);
+				if(sfwrite(outfile,stakptr(staktell()),n) < 0)
+					exitval = 1;
 		}
 		else
-			sfputr(outfile,cp,-1);
+			if(sfputr(outfile,cp,-1) < 0)
+				exitval = 1;
 		if(*argv)
-			sfputc(outfile,' ');
+			if(sfputc(outfile,' ') < 0)
+				exitval = 1;
 		sh_sigcheck(shp);
 	}
 	return(!pdata.cescape);
@@ -633,7 +634,8 @@ static ssize_t fmtbase64(Sfio_t *iop, char *string, int alt)
 	else if(nv_isarray(np) && (ap=nv_arrayptr(np)) && array_elem(ap) && (ap->nelem&(ARRAY_UNDEF|ARRAY_SCAN)))
 	{
 		nv_outnode(np,iop,(alt?-1:0),0);
-		sfputc(iop,')');
+		if(sfputc(iop,')') < 0)
+			exitval = 1;
 		return(sftell(iop));
 	}
 	else

src/cmd/ksh93/include/version.h

@@ -21,7 +21,7 @@
 
 #define SH_RELEASE_FORK	"93u+m"		/* only change if you develop a new ksh93 fork */
 #define SH_RELEASE_SVER	"1.0.0-beta.2"	/* semantic version number: https://semver.org */
-#define SH_RELEASE_DATE	"2021-09-13"	/* must be in this format for $((.sh.version)) */
+#define SH_RELEASE_DATE	"2021-11-07"	/* must be in this format for $((.sh.version)) */
 #define SH_RELEASE_CPYR	"(c) 2020-2021 Contributors to ksh " SH_RELEASE_FORK
 
 /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

src/cmd/ksh93/sh/main.c

@@ -344,6 +344,10 @@ int sh_main(int ac, char *av[], Shinit_f userinit)
 				sh_accbegin(error_info.id);
 #endif	/* SHOPT_ACCT */
 		}
+		/* If the shell is init'ed with std{in,out,err} closed, make the shell's FD state reflect that. */
+		for(i=0; i<=2; i++)
+			if(fcntl(i,F_GETFD,NiL)==-1 && errno==EBADF)	/* closed at OS level? */
+				sh_close(i); 				/* update shell FD state */
 	}
 	else
 	{

src/cmd/ksh93/tests/io.sh

@@ -878,5 +878,26 @@ got=$(< dir1/dir2/x)
 [[ $got == "$exp" ]] || err_exit "symlink in conditional redirect wrong" \
 	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 
+# ======
+# ksh misbehaved when stdout is closed
+# https://github.com/ksh93/ksh/issues/314
+"$SHELL" -c 'pwd; echo "$?" >&2; echo test; echo "$?" > file' >&- 2>stderr
+exp='1'
+[[ $(<file) == "$exp" ]] || err_exit "ksh misbehaves when stdout is closed (1)" \
+	"(expected $(printf %q "$exp"), got $(printf %q "$(<file)"))"
+exp='0'
+[[ $(<stderr) == "$exp" ]] || err_exit "ksh misbehaves when stdout is closed (2)" \
+	"(expected $(printf %q "$exp"), got $(printf %q "$(<stderr)"))"
+for cmd in echo print printf
+do	"$cmd" hi >&- && err_exit "'$cmd' does not detect closed stdout (simple redirection)"
+	"$SHELL" -c "$cmd hi" >&- && err_exit "'$cmd' does not detect closed stdout (inherited FD)"
+done
+if	[[ -c /dev/full ]]
+then	for cmd in echo print printf
+	do	"$cmd" hi >/dev/full && err_exit "'$cmd' does not detect disk full (simple redirection)"
+		"$SHELL" -c "$cmd hi" >/dev/full && err_exit "'$cmd' does not detect disk full (inherited FD)"
+	done
+fi
+
 # ======
 exit $((Errors<125?Errors:125))