In article <20070604183552.GN26767@canolog.ninthwonder.com>,
Allen Briggs  <briggs@netbsd.org> wrote:
>-=-=-=-=-=-
>
>I found that the buffersize passed to progress can make a pretty
>sizable different in speed -- at least on a 'dd' test.  The 1k
>(BUFSIZ) default buffer size might be handy for really tight
>memory requirements, but a larger buffer size gets better pipe
>throughput.
>
>So I made a few changes (make the buffer dynamically allocated,
>make the buffer size a command-line option, and default the buffer
>size to 64k) that I'd like to propose.
>
>Diffs attached.  Comments welcome.
>
>-allen
>
>-- 
>Allen Briggs  |  http://www.ninthwonder.com/~briggs/  |  briggs@ninthwonder.com
>
>-=-=-=-=-=-
>
>? .gdbinit
>? progress
>? progress.cat1
>Index: progress.1
>===================================================================
>RCS file: /cvsroot/src/usr.bin/progress/progress.1,v
>retrieving revision 1.12
>diff -p -u -r1.12 progress.1
>--- progress.1	12 Apr 2007 06:31:20 -0000	1.12
>+++ progress.1	4 Jun 2007 18:30:02 -0000
>@@ -30,7 +30,7 @@
> .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> .\" POSSIBILITY OF SUCH DAMAGE.
> .\"
>-.Dd April 12, 2007
>+.Dd June 4, 2008
> .Dt PROGRESS 1
> .Os
> .Sh NAME
>@@ -39,6 +39,7 @@
> .Sh SYNOPSIS
> .Nm
> .Op Fl ez
>+.Op Fl b Ar buffersize
> .Op Fl f Ar file
> .Op Fl l Ar length
> .Op Fl p Ar prefix
>@@ -65,6 +66,11 @@ simply displays a count of the data and 
> .Pp
> The options are as follows:
> .Bl -tag -width XlXlengthXX
>+.It Fl b Ar buffersize
>+Read in buffers of the specified size (default 64k).
>+An optional suffix (per
>+.Xr strsuftoll 3 )
>+may be given.
> .It Fl e
> Display progress to standard error instead of standard output.
> .It Fl f Ar file
>Index: progress.c
>===================================================================
>RCS file: /cvsroot/src/usr.bin/progress/progress.c,v
>retrieving revision 1.14
>diff -p -u -r1.14 progress.c
>--- progress.c	7 Feb 2007 15:21:21 -0000	1.14
>+++ progress.c	4 Jun 2007 18:30:02 -0000
>@@ -73,8 +73,9 @@ static void
> usage(void)
> {
> 	fprintf(stderr,
>-	    "usage: %s [-ez] [-f file] [-l length] [-p prefix] cmd [args...]\n",
>-	    getprogname());
>+	    "usage: %s [-ez] [-b buffersize] [-f file] [-l length]\n"
>+	    "       %*.s [-p prefix] cmd [args...]\n",
>+	    getprogname(), (int) strlen(getprogname()), "");

Why %*.s? we never do this...

> 	exit(EXIT_FAILURE);
> }
> 
>@@ -82,7 +83,8 @@ usage(void)
> int
> main(int argc, char *argv[])
> {
>-	static char fb_buf[BUFSIZ];
>+	long long buffersize;

Why make buffersize long long, since malloc can only handle size_t?
Or at least check that buffersize < SIZE_T_MAX.

>+	char *fb_buf;
> 	char *infile = NULL;
> 	pid_t pid = 0, gzippid = 0, deadpid;
> 	int ch, fd, outpipe[2];
>@@ -97,10 +99,16 @@ main(int argc, char *argv[])
> 	/* defaults: Read from stdin, 0 filesize (no completion estimate) */
> 	fd = STDIN_FILENO;
> 	filesize = 0;
>+	buffersize = 64 * 1024;
> 	prefix = NULL;
> 
>-	while ((ch = getopt(argc, argv, "ef:l:p:z")) != -1)
>+	while ((ch = getopt(argc, argv, "b:ef:l:p:z")) != -1)
> 		switch (ch) {
>+		case 'b':
>+			/* 256MB seems absurdly large */
>+			buffersize = strsuftoll("buffer size", optarg,
>+			    0, 256 * 1024 * 1024 );

no space before paren.

>+			break;
> 		case 'e':
> 			eflag++;
> 			break;
>@@ -202,6 +210,10 @@ main(int argc, char *argv[])
> 	else
> 		ttywidth = ts.ts_cols;
> 
>+	fb_buf = malloc(buffersize);
>+	if (fb_buf == NULL)
>+		err(1, "malloc for buffersize");
>+
> 	if (pipe(outpipe) < 0)
> 		err(1, "output pipe");
> 	pid = fork();
>@@ -219,7 +231,7 @@ main(int argc, char *argv[])
> 	close(outpipe[0]);
> 
> 	progressmeter(-1);
>-	while ((nr = read(fd, fb_buf, BUFSIZ)) > 0)
>+	while ((nr = read(fd, fb_buf, buffersize)) > 0)
> 		for (off = 0; nr; nr -= nw, off += nw, bytes += nw)
> 			if ((nw = write(outpipe[1], fb_buf + off,
> 			    (size_t) nr)) < 0)
>@@ -255,5 +267,8 @@ main(int argc, char *argv[])
> 	}
> 
> 	progressmeter(1);
>+
>+	free(fb_buf);
>+
> 	exit(cmdstat ? cmdstat : gzipstat);
> }
>
>-=-=-=-=-=-