[comp.os.minix] Fixing 7K pipes again

msm@attctc.Dallas.TX.US (Michael Mattone) (09/28/89)

A few weeks ago, Bruce Evans (evans@ditsyda.oz.au), mentioned a
fix I had posted about a year ago that was supposed to fix the
limit of 7K when writing to a pipe.  He also mentioned that it
had a bug that caused blocks to be skipped.  Well, I took a look
at FS with my patches again, and believe I have fixed this bug.

I believe the bug can be fixed by adding one line to the fs/read.c
file before the "goto bigpipe" statement (from my original posting):

	if (nbytes > xbytes) {
		f->filp_pos = position;	/* BUG fixed - add this line */
		goto bigpipe;		/* more in a pipe to do */
	}

I'm enclosing my patches again in case there are others interested.
I'd really like to hear if anyone tries this and of any other bugs they
might find.

----------------------------------cut here---------------------------------
echo x - fproc.h.cdiff
sed '/^X/s///' > fproc.h.cdiff << '/'
X*** fproc.h	Tue Jun 28 22:28:05 1988
X--- new/fproc.h	Mon Aug 15 02:12:25 1988
X***************
X*** 22,27 ****
X--- 22,28 ----
X    char fp_task;			/* which task is proc suspended on */
X    int fp_pid;			/* process id */
X    int fp_pgrp;			/* process group */
X+   int fp_tot_io;		/* total cum_io for pipes */
X  } fproc[NR_PROCS];
X  
X  /* Field values. */
/
echo x - main.c.cdiff
sed '/^X/s///' > main.c.cdiff << '/'
X*** main.c	Tue Jun 28 22:28:13 1988
X--- new/main.c	Mon Aug 15 02:08:10 1988
X***************
X*** 110,115 ****
X--- 110,116 ----
X  
X    who = m.m_source;
X    fs_call = m.m_type;
X+   fproc[who].fp_tot_io = 0;		/* in case we do a pipe */
X  }
X  
X  
/
echo x - pipe.c.cdiff
sed '/^X/s///' > pipe.c.cdiff << '/'
X*** pipe.c	Sat Jul 16 20:34:19 1988
X--- new/pipe.c	Tue Nov  1 01:26:47 1988
X***************
X*** 21,26 ****
X--- 21,27 ----
X  #include "../h/signal.h"
X  #include "const.h"
X  #include "type.h"
X+ #include "dev.h"
X  #include "file.h"
X  #include "fproc.h"
X  #include "glo.h"
X***************
X*** 81,91 ****
X  /*===========================================================================*
X   *				pipe_check				     *
X   *===========================================================================*/
X! PUBLIC int pipe_check(rip, rw_flag, bytes, position)
X  register struct inode *rip;	/* the inode of the pipe */
X  int rw_flag;			/* READING or WRITING */
X  register int bytes;		/* bytes to be read or written (all chunks) */
X  register file_pos position;	/* pointer to current file position */
X  {
X  /* Pipes are a little different.  If a process reads from an empty pipe for
X   * which a writer still exists, suspend the reader.  If the pipe is empty
X--- 82,93 ----
X  /*===========================================================================*
X   *				pipe_check				     *
X   *===========================================================================*/
X! PUBLIC int pipe_check(rip, rw_flag, bytes, position, xbytes)
X  register struct inode *rip;	/* the inode of the pipe */
X  int rw_flag;			/* READING or WRITING */
X  register int bytes;		/* bytes to be read or written (all chunks) */
X  register file_pos position;	/* pointer to current file position */
X+ int  *xbytes;
X  {
X  /* Pipes are a little different.  If a process reads from an empty pipe for
X   * which a writer still exists, suspend the reader.  If the pipe is empty
X***************
X*** 110,122 ****
X  	}
X    } else {
X  	/* Process is writing to a pipe. */
X- 	if (bytes > PIPE_SIZE) return(EFBIG);
X  	if (find_filp(rip, R_BIT) == NIL_FILP) {
X  		/* Tell kernel to generate a SIGPIPE signal. */
X  		sys_kill((int)(fp - fproc), SIGPIPE);
X  		return(EPIPE);
X  	}
X  
X  	if (position + bytes > PIPE_SIZE) {
X  		suspend(XPIPE);	/* stop writer -- pipe full */
X  		return(0);
X--- 112,128 ----
X  	}
X    } else {
X  	/* Process is writing to a pipe. */
X  	if (find_filp(rip, R_BIT) == NIL_FILP) {
X  		/* Tell kernel to generate a SIGPIPE signal. */
X  		sys_kill((int)(fp - fproc), SIGPIPE);
X  		return(EPIPE);
X  	}
X  
X+ 	if (bytes > PIPE_SIZE) {
X+ 		if ((*xbytes = PIPE_SIZE - position) > 0)
X+ 			bytes = *xbytes;
X+ 	}
X+ 
X  	if (position + bytes > PIPE_SIZE) {
X  		suspend(XPIPE);	/* stop writer -- pipe full */
X  		return(0);
/
echo x - read.c.cdiff
sed '/^X/s///' > read.c.cdiff << '/'
X*** read.c	Fri Sep 30 03:15:42 1988
X--- new/read.c	Wed Sep 27 19:23:04 1989
X***************
X*** 54,59 ****
X--- 54,60 ----
X    register unsigned off, cum_io;
X    file_pos position;
X    int r, chunk, mode_word, usr, seg, block_spec, char_spec;
X+   int  xbytes;
X    struct filp *wf;
X    extern struct super_block *get_super();
X    extern struct filp *find_filp(), *get_filp();
X***************
X*** 111,124 ****
X  		if (position > f_size) clear_zone(rip, f_size, 0);
X  	}
X  
X! 	/* Pipes are a little different.  Check. */
X! 	if (rip->i_pipe &&
X! 	    (r = pipe_check(rip, rw_flag, nbytes, position)) <= 0)
X! 		return r;
X! 	/* Split the transfer into chunks that don't span two blocks. */
X! 	while (nbytes != 0) {
X! 		off = position % BLOCK_SIZE;	/* offset within a block */
X! 		chunk = MIN(nbytes, BLOCK_SIZE - off);
X  		if (chunk < 0) chunk = BLOCK_SIZE - off;
X  
X  		if (rw_flag == READING || (block_spec && rw_flag == WRITING)) {
X--- 112,127 ----
X  		if (position > f_size) clear_zone(rip, f_size, 0);
X  	}
X  
X! bigpipe:
X! 	xbytes = nbytes;
X! 	/* Pipes are a little different.  Check. */
X! 	if (rip->i_pipe &&
X! 	    (r = pipe_check(rip, rw_flag, nbytes, position, &xbytes)) <= 0)
X! 		return r;
X! 	/* Split the transfer into chunks that don't span two blocks. */
X! 	while (xbytes != 0) {
X! 		off = position % BLOCK_SIZE;	/* offset within a block */
X! 		chunk = MIN(xbytes, BLOCK_SIZE - off);
X  		if (chunk < 0) chunk = BLOCK_SIZE - off;
X  
X  		if (rw_flag == READING || (block_spec && rw_flag == WRITING)) {
X***************
X*** 136,141 ****
X--- 139,145 ----
X  		/* Update counters and pointers. */
X  		buffer += chunk;	/* user buffer address */
X  		nbytes -= chunk;	/* bytes yet to be read */
X+ 		xbytes -= chunk;	/* for pipes */
X  		cum_io += chunk;	/* bytes read so far */
X  		position += chunk;	/* position within the file */
X  	}
X***************
X*** 148,153 ****
X--- 152,165 ----
X  		rip->i_modtime = clock_time();
X  		rip->i_dirt = DIRTY;
X  	}
X+ 	if (r == OK && rip->i_pipe) {
X+ 		fp->fp_tot_io += cum_io;
X+ 		if (nbytes > xbytes) {
X+ 			f->filp_pos = position;	/* just in case */
X+ 			goto bigpipe;		/* more in a pipe to do */
X+ 		}
X+ 		cum_io = fp->fp_tot_io;		/* for accurate count */
X+ 	}
X    } else {
X  	if (rip->i_pipe && position >= rip->i_size) {
X  		/* Reset pipe pointers. */
/

ast@cs.vu.nl (Andy Tanenbaum) (09/28/89)

In article <9507@attctc.Dallas.TX.US> msm@attctc.Dallas.TX.US (Michael Mattone) writes:
>		goto bigpipe;		/* more in a pipe to do */

This is definitely a nono.  No gotos please!

Andy Tanenbaum (ast@cs.vu.nl)

mccrea@ecf.ncsl.nist.gov (MCCREA, SAM) (09/28/89)

>>      goto bigpipe;       /* more in a pipe to do */

That is definitely a YES, YES!!

Where did you get your programming instruction? From
a Cereal box?

tweten@gilmore.nas.nasa.gov (Dave Tweten) (09/29/89)

From: Andy Tanenbaum <ast%CS.VU.NL@VM1.NoDak.EDU>

	This is definitely a nono.  No gotos please!

I hope you are merely overstating you case, Dr. Tanenbaum.  I had
thought the "No gotos" religion had died, mercifully for the rest of
us.

I continue to find that something on the order of one "goto" per 5000
lines of C code or so is required to maximize simplicity, clarity and
therefore maintainability.  Applications include breaking out of two
levels of loop at once, breaking out of a loop, but not executing the
loop exhaustion code, switch cases which should decide to break out of
the surrounding loop, etc., etc.  I didn't see enough context to tell
if the specific "goto" which triggered your message was a good one, but
surely an absolute "No gotos" is not a defensable position.

Please say you were kidding.

ast@cs.vu.nl (Andy Tanenbaum) (09/30/89)

In article <24962@louie.udel.EDU> tweten@gilmore.nas.nasa.gov (Dave Tweten) writes:
>From: Andy Tanenbaum <ast%CS.VU.NL@VM1.NoDak.EDU>
>surely an absolute "No gotos" is not a defensable position.
I'll bet you like

    i = 0;
     <<VAX
      some VAX assembly statements
         XAV>>
     j=0;
     more C

Andy Tanenbaum (ast@cs.vu.nl)

cs304pal@rata.vuw.ac.nz (Lloyd Parkes) (10/02/89)

In article <24962@louie.udel.EDU> tweten@gilmore.nas.nasa.gov (Dave Tweten) writes:

>I hope you are merely overstating you case, Dr. Tanenbaum.  I had
>thought the "No gotos" religion had died, mercifully for the rest of
>us.

I have only been programming in C for a couple of years now, but I have
been programming for many years before that. I have written most sorts of
programs, including an assembler, a heap management program and a multitude
of file handling programs. And I can quite truthfully say that since I left
BASIC, I have *NEVER* needed a goto. There is always (I hope) a way to
write a piece of code without resorting to gotos. If you can't get a
readable program without using a goto, then you need to rethink, it might
be a major rethink, but there is always a way.

						Lloyd

Quick, send your money to cs304pal@rata.vuw.ac.nz now!

If you think anyone believes what I have just said,
then you must be daft in the head!