Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 5 Mar 2017 13:50:26 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Conrad Meyer <cem@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r314685 - head/bin/ps
Message-ID:  <20170305115437.R1556@besplex.bde.org>
In-Reply-To: <201703042238.v24McAD8008837@repo.freebsd.org>
References:  <201703042238.v24McAD8008837@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 4 Mar 2017, Conrad Meyer wrote:

> Author: cem
> Date: Sat Mar  4 22:38:10 2017
> New Revision: 314685
> URL: https://svnweb.freebsd.org/changeset/base/314685
>
> Log:
>  ps(1): Only detect terminal width if stdout is a tty
>
>  If stdout isn't a tty, use unlimited width output rather than truncating to
>  79 characters.  This is helpful for shell scripts or e.g., 'ps | grep foo'.

This breaks many interactive uses, e.g., 'ps l | less', to work around a
user error.

If the user wants an unlimited width, then they should have asked for it
using -ww (-w only increases the width to 131 if it was smaller).

Shell scripts should be especially careful to use the right options...

>  This hardcoded width has some history: In The Beginning of History[0], the
>  width of ps was hardcoded as 80 bytes.  In 1985, Bloom@ added detection
>  using TIOCGWINSZ on stdin.[1]  In 1986, Kirk merged a change to check
>  stdout's window size instead.  In 1990, the fallback checks to stderr and
>  stdin's TIOCGWINSZ were added by Marc@, with the commit message "new
>  version."[2]
>
>  OS X Darwin has a very similar modification to ps(1), which simply sets
>  UNLIMITED for all non-tty outputs.[3]  I've chosen to respect COLUMNS
>  instead of behaving identically to Darwin here, but I don't feel strongly
>  about that.  We could match OS X for parity if that is desired.

Ignoring COLUMNS would break it completely.  COLUMNS is the only way to
control the width.  Interactive use normally wants the width of the current
terminal, but shell scripts might want to create a file to be read on a
different terminal, or just back on the same terminal where the script
cannout know about the current terminal because it is purely non-interactive
(see comments below about checking all 3 std fd's consistently).

>  [0]: https://svnweb.freebsd.org/csrg/bin/ps/ps.c?annotate=1065
>  [1]: https://svnweb.freebsd.org/csrg/bin/ps/ps.c?r1=18105&r2=18106
>  [2]:
>  https://svnweb.freebsd.org/csrg/bin/ps/ps.c?r1=40675&r2=40674&pathrev=40675
>  [3]:
>  https://opensource.apple.com/source/adv_cmds/adv_cmds-168/ps/ps.c.auto.html
>
>  PR:		217159
>  Reported by:	Deepak Nagaraj <n.deepak at gmail.com>
>
> Modified:
>  head/bin/ps/ps.c
>
> Modified: head/bin/ps/ps.c
> ==============================================================================
> --- head/bin/ps/ps.c	Sat Mar  4 22:23:59 2017	(r314684)
> +++ head/bin/ps/ps.c	Sat Mar  4 22:38:10 2017	(r314685)
> @@ -194,6 +194,8 @@ main(int argc, char *argv[])
>
> 	if ((cols = getenv("COLUMNS")) != NULL && *cols != '\0')
> 		termwidth = atoi(cols);
> +	else if (!isatty(STDOUT_FILENO))
> +		termwidth = UNLIMITED;
> 	else if ((ioctl(STDOUT_FILENO, TIOCGWINSZ, (char *)&ws) == -1 &&
> 	     ioctl(STDERR_FILENO, TIOCGWINSZ, (char *)&ws) == -1 &&
> 	     ioctl(STDIN_FILENO,  TIOCGWINSZ, (char *)&ws) == -1) ||

For use in shell scripts, the user should know too much about COLUMNS.
COLUMNS is set automatically by some shells, but not exported, so it
is not normally set here.  But shell scripts should worry about the
effects of environment variables.  Hard-coded -ww would cancel the
setting of termwidth here.  This is the easiest way to blow away the
user's setting  of COLUMNS.  Security-related scripts should do this.
But most scripts should honor the setting.  The use might actually
want the setting to affect ps in the script.

It is probably wrong to check only stdout.  ps is only clearly not
associated with a terminal if all 3 std fd's are not terminals.

If there was a bug in the old version, then it was that the default
is chosen too magically, so no one can remember how to control it.

   (Even simple use of isatty() gives that problem, and causes a
   larger problem in practice -- stdio gives line-buffered output
   which is best for interactive use, but as soon as you pipe the
   output though less it becomes fully buffered and is often delayed
   too long.  Some utilities have an option to give line-buffered
   output (e.g., -l for tcpdump), but most don't, and I can rarely
   remember if they do.)

The problem is not really different for shell scripts.  ps in a shell
script still uses the terminal width if the script outputs to a
terminal.  But if you pipe through less so that you can actually read
the output before it scrolls off, then it magically becomes wide,
unless you remember the COLUMNS variable, and export it, and the script
honors it.  This shows why it is indeed wrong to not check all 3 std
fd's.  Both 'ps | less' and 'script-doing-ps | less' are interactive
uses if they are started on a terminal, since stdin is still a terminal
and so is stderr since you forgot to pipe it to less.  The width should
not change in this case.

   (stdio only does isatty() on the fd of the current stream to set the
   default for line buffering.  It would be too magical for it to check
   other fd's, especially when the current fd is not a std one.  I think
   the unwanted full buffering for most utilities when they are piped
   through less to a terminal results from this.  They just use stdio,
   and it doesn't know the context, so it correctly doesn't adjust the
   magic.  Then most utilities don't know about using setvbuf() to
   adjust the magic to their context, and they really shouldn't know
   if the output is a pipe...)

top(1) has a related non-problem with the default length.  For the non-
interactive use 'top >file', you normally want to see more lines than
fit on the terminal and a probably doing this to see them.  But
the length doesn't change by default.  It is a user error to not use
the option to change it.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170305115437.R1556>