Re: stdio buffering extension

[David Brownlee <abs%netbsd.org@localhost>]

On 14 July 2015 at 21:21, Christos Zoulas <christos%zoulas.com@localhost> wrote:
>
> Hi,
>
> After discussing importing stdbuf(1) from FreeBSD with various
> people, they objected about it using LD_PRELOAD and suggested to
> use the environment directly to alter the default buffering policy.
>
> Here's an implementation of that...
>
> Suggestions/Comments?
>
> christos
>
> Index: fopen.3
> ===================================================================
> RCS file: /cvsroot/src/lib/libc/stdio/fopen.3,v
> retrieving revision 1.30
> diff -u -u -r1.30 fopen.3
> --- fopen.3     11 Feb 2015 15:19:05 -0000      1.30
> +++ fopen.3     14 Jul 2015 20:16:45 -0000
> @@ -196,6 +196,13 @@
>  .Em stdin ,
>  or
>  .Em stdout ) .
> +.Pp
> +Input and output against the opened stream will be fully buffered, unless
> +it refers to an interactive terminal device, or a different kind of buffering
> +is specified in the environment.
> +See
> +.Xr setvbuf 3
> +for additional details.
>  .Sh RETURN VALUES
>  Upon successful completion
>  .Fn fopen ,
> Index: makebuf.c
> ===================================================================
> RCS file: /cvsroot/src/lib/libc/stdio/makebuf.c,v
> retrieving revision 1.17
> diff -u -u -r1.17 makebuf.c
> --- makebuf.c   15 Mar 2012 18:22:30 -0000      1.17
> +++ makebuf.c   14 Jul 2015 20:16:45 -0000
> @@ -49,10 +49,64 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <unistd.h>
> +#include <inttypes.h>
> +#include <ctype.h>
>  #include "reentrant.h"
>  #include "local.h"
>
>  /*
> + * Override the file buffering based on the environment setting STDBUF%d
> + * (for the specific file descriptor) and STDBUF (for all descriptors).
> + * the setting is ULB<num> standing for "Unbuffered", "Linebuffered",
> + * and Fullybuffered", and <num> is a value from 0 to 1M
> + */
> +static int
> +__senvbuf(FILE *fp, size_t *size, int *couldbetty)
> +{
> +       char evb[64], *evp;
> +       int flags, e;
> +       intmax_t s;
> +
> +       flags = 0;
> +       if (snprintf(evb, sizeof(evb), "STDBUF%d", fp->_file) < 0)
> +               return flags;
> +
> +       if ((evp = getenv(evb)) == NULL && (evp = getenv("STDBUF")) == NULL)
> +               return flags;
> +
> +       switch (*evp) {
> +       case 'u':
> +       case 'U':
> +               evp++;
> +               flags |= __SNBF;
> +               break;
> +       case 'l':
> +       case 'L':
> +               evp++;
> +               flags |= __SLBF;
> +               break;
> +       case 'f':
> +       case 'F':
> +               evp++;
> +               *couldbetty = 0;
> +               break;
> +       }
> +
> +       if (!isdigit((unsigned char)*evp))
> +               return flags;
> +
> +       s = strtoi(evp, NULL, 0, 0, 1024 * 1024, &e);
> +       if (e != 0)
> +               return flags;
> +
> +       *size = (size_t)s;
> +       if (*size == 0)
> +               return __SNBF;
> +
> +       return flags;
> +}
> +
> +/*
>   * Allocate a file buffer, or switch to unbuffered I/O.
>   * Per the ANSI C standard, ALL tty devices default to line buffered.
>   *
> @@ -69,18 +123,21 @@
>
>         _DIAGASSERT(fp != NULL);
>
> -       if (fp->_flags & __SNBF) {
> -               fp->_bf._base = fp->_p = fp->_nbuf;
> -               fp->_bf._size = 1;
> -               return;
> -       }
> +       if (fp->_flags & __SNBF)
> +               goto unbuf;
> +
>         flags = __swhatbuf(fp, &size, &couldbetty);
> -       if ((p = malloc(size)) == NULL) {
> -               fp->_flags |= __SNBF;
> -               fp->_bf._base = fp->_p = fp->_nbuf;
> -               fp->_bf._size = 1;
> -               return;
> +
> +       if ((fp->_flags & (__SLBF|__SNBF|__SMBF)) == 0
> +           && fp->_cookie == fp && fp->_file >= 0) {
> +               flags |= __senvbuf(fp, &size, &couldbetty);
> +               if (flags & __SNBF)
> +                       goto unbuf;
>         }
> +
> +       if ((p = malloc(size)) == NULL)
> +               goto unbuf;
> +
>         __cleanup = _cleanup;
>         flags |= __SMBF;
>         fp->_bf._base = fp->_p = p;
> @@ -89,6 +146,11 @@
>         if (couldbetty && isatty(__sfileno(fp)))
>                 flags |= __SLBF;
>         fp->_flags |= flags;
> +       return;
> +unbuf:
> +       fp->_flags |= __SNBF;
> +       fp->_bf._base = fp->_p = fp->_nbuf;
> +       fp->_bf._size = 1;
>  }
>
>  /*
> Index: setbuf.3
> ===================================================================
> RCS file: /cvsroot/src/lib/libc/stdio/setbuf.3,v
> retrieving revision 1.13
> diff -u -u -r1.13 setbuf.3
> --- setbuf.3    7 Aug 2003 16:43:31 -0000       1.13
> +++ setbuf.3    14 Jul 2015 20:16:45 -0000
> @@ -63,6 +63,27 @@
>  when it is line buffered characters are saved up until a newline is
>  output or input is read from any stream attached to a terminal device
>  (typically stdin).
> +.Pp
> +The default buffer settings can be overwritten per descriptor
> +.Dv ( STDBUFn )
> +where
> +.Dv n
> +is the numeric value of the file descriptor represented by the stream, or
> +for all descriptors
> +.Dv ( STDBUF ) .
> +The environment variable value is a letter followed by an optional numeric
> +value indicating the size of the buffer.
> +Valid sizes range from 0B to 1MB.
> +Valid letters are:
> +.Bl -tag -width X -indent
> +.It Dv Li U
> +Unbuffered.
> +.It Dv Li L
> +Line-buffered.
> +.It Dv Li F
> +Fully-buffered.
> +.El
> +.Pp
>  The function
>  .Xr fflush 3
>  may be used to force the block out early.

I'd love to see this in (and used in rc :)

Some possible thoughts:

Do we want to disallow this for setuid programs (probably not an
issue, but just in case). Would the 1M limit be good to take from a
sysctl value to allow tuning safe limits (thinking of a resource DOS
on a small system, though someone needs to be able to set arbitrary
environment variables anyway)