more problems w/ irix

more problems w/ irix

[Ray Jones]

i applied pws's patch, and it fixed the fdtable problems for the cases
that i was seeing a crash before.  

to test it further and see how it responds when given more files than
OPEN_MAX, i tried the "cat < * > /dev/null" in a directory w/ about
4000 files (OPEN_MAX is 2500).  now i'm getting a crash in halloc(),
or (if i turn enable zsh_mem) malloc().  the call stack for the second
case is:

   0 malloc(size = 8188) ["zsh-3.0.1/Src/mem.c":613, 0x45fd04]
   1 zalloc(size = 8188) ["zsh-3.0.1/Src/mem.c":315, 0x45f500]
   2 halloc(size = 12) ["zsh-3.0.1/Src/mem.c":259, 0x45f178]
   3 hcalloc(size = 12) ["zsh-3.0.1/Src/mem.c":289, 0x45f3f4]
   4 insertlinknode(list = 0x1007d924, node = 0x1007d924, dat = 0x1001f028) 
	["zsh-3.0.1/Src/linklist.c":57, 0x458ec8]
   5 xpandredir(fn = 0x10054eec, tab = 0x1001de18) 
	["zsh-3.0.1/Src/glob.c":870, 0x438278]
[...]

this is 3.0.1 out of the box, with the aforementioned patch, on irix
5.3.  it might be the case that there's already been a patch that
fixes this.  if so, i apologize.

thanks,
ray jones

Re: more problems w/ irix

[Bart Schaefer]

On Nov 13, 10:23am, Ray Jones wrote:
} Subject: more problems w/ irix
}
} to test it further and see how it responds when given more files than
} OPEN_MAX, i tried the "cat < * > /dev/null" in a directory w/ about
} 4000 files (OPEN_MAX is 2500).  now i'm getting a crash in halloc(),
} or (if i turn enable zsh_mem) malloc().

Different bug, I suspect.  Note that "cat < *" is the same as writing
"cat < file1 < file2 < file3 < file4 ..." and is in fact converted by
xpandredir() [glob.c] to exactly the same syntax tree that you'd get
if you typed out all the redirections explicitly.

Later on, in execcmd() [exec.c], when the redirections are processed,
there is no check done to be sure that there are not more redirections
attempted than the current value of OPEN_MAX (fdtable_size after PWS's
patch).  There *is* a test for whether open() returns failure, but then
in addfd() zsh pretty much ignores failures of dup() via movefd(); and
addfd() is itself void, so execcmd() has no chance.

It ould be the case that IRIX doesn't obey its own limits on number of
open files -- if open() or dup() continues to succeed even after the
soft limit on number of files is reached, zsh happily continues adding
fds beyond the end of fdtable[], scribbling on random heap.

You should try compiling with --enable-zsh-debug and watch for Peter's
"fdtable too small" warnings.  I bet you'll get them.

If you do get them, then the fdtable *does* need to grow dynamically
as fds are added to it; it can't be preallocated at the time limits are
examined.  But we don't know that for sure yet.

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

Re: more problems w/ irix

[rjones]

schaefer@nbn.com wrote:
> You should try compiling with --enable-zsh-debug and watch for Peter's
> "fdtable too small" warnings.  I bet you'll get them.

nope... never saw them.  i thought that was probably the cause, but it
seems to be something else.

ray jones

Re: more problems w/ irix

[Peter Stephenson]

Ray Jones wrote:
> i applied pws's patch, and it fixed the fdtable problems for the cases
> that i was seeing a crash before.  
> 
> to test it further and see how it responds when given more files than
> OPEN_MAX, i tried the "cat < * > /dev/null" in a directory w/ about
> 4000 files (OPEN_MAX is 2500).  now i'm getting a crash in halloc(),
> or (if i turn enable zsh_mem) malloc().

I have a horrible feeling all that OPEN_MAX stuff was actually
irrelevant... struct multio can only hold MAXREDIRECT redirections,
normally 16; trying <* where there are more than 16 expanded files
simply isn't supposed to work.  I've added the test and now it won't
work.  I've documented this.

If <muffled but entirely audible whimper> anyone wants to remove the
limit, it would be possible to make the fds member a pointer, and
enlarge it as necessary.  I have little enthusiasm.  It's a rather
inefficient way of processing a lot of files anyway.

*** Doc/zshmisc.man.mfd	Sat Aug 31 15:24:25 1996
--- Doc/zshmisc.man	Thu Nov 14 10:58:24 1996
***************
*** 423,429 ****
  assuming there's at least one.
  (Without the
  .B MULTIOS
! option, it would create an empty file called "*".)
  .PP
  If the user tries to open a file descriptor for reading more than once,
  the shell opens the file descriptor as a pipe to a process that copies
--- 423,431 ----
  assuming there's at least one.
  (Without the
  .B MULTIOS
! option, it would create an empty file called "*".)  Note that the
! number of multiple redirections is restricted to 16 for a particular
! file descriptor.
  .PP
  If the user tries to open a file descriptor for reading more than once,
  the shell opens the file descriptor as a pipe to a process that copies
*** Src/exec.c.mfd	Thu Nov 14 10:42:14 1996
--- Src/exec.c	Thu Nov 14 10:44:28 1996
***************
*** 1047,1054 ****
  	    mfds[fd1]->pipe = pipes[1 - rflag];
  	    redup(pipes[rflag], fd1);
  	    mfds[fd1]->ct = 2;
! 	} else			/* add another fd to an already split stream */
  	    mfds[fd1]->fds[mfds[fd1]->ct++] = movefd(fd2);
      }
      if (subsh_close >= 0 && !fdtable[subsh_close])
  	subsh_close = -1;
--- 1047,1057 ----
  	    mfds[fd1]->pipe = pipes[1 - rflag];
  	    redup(pipes[rflag], fd1);
  	    mfds[fd1]->ct = 2;
! 	} else if (mfds[fd1]->ct < MAXREDIRECT) {
! 	    /* add another fd to an already split stream */
  	    mfds[fd1]->fds[mfds[fd1]->ct++] = movefd(fd2);
+ 	} else			/* run out of space in struct multio */
+ 	    close(fd2);
      }
      if (subsh_close >= 0 && !fdtable[subsh_close])
  	subsh_close = -1;

-- 
Peter Stephenson <pws@ifh.de>       Tel: +49 33762 77366
WWW:  http://www.ifh.de/~pws/       Fax: +49 33762 77413
Deutches Electronen-Synchrotron --- Institut fuer Hochenergiephysik Zeuthen
DESY-IfH, 15735 Zeuthen, Germany.

Re: more problems w/ irix

[Zefram]

>I have a horrible feeling all that OPEN_MAX stuff was actually
>irrelevant... struct multio can only hold MAXREDIRECT redirections,
>normally 16; trying <* where there are more than 16 expanded files
>simply isn't supposed to work.  I've added the test and now it won't
>work.  I've documented this.

I've been meaning to do something about that for a while now.

>If <muffled but entirely audible whimper> anyone wants to remove the
>limit, it would be possible to make the fds member a pointer, and
>enlarge it as necessary.

I think the neatest way to handle it may be the struct hack, as the
multios are already malloced.

>                          I have little enthusiasm.  It's a rather
>inefficient way of processing a lot of files anyway.

It's a nice syntax.  If it's grossly inefficient, we should improve the
code; being built in to the shell it *ought* to be more efficient than
execing cat, at least for small files.

-zefram

Re: more problems w/ irix

[Richard Coleman]

> >If <muffled but entirely audible whimper> anyone wants to remove the
> >limit, it would be possible to make the fds member a pointer, and
> >enlarge it as necessary.
> 
> I think the neatest way to handle it may be the struct hack, as the
> multios are already malloced.
> 
> >                          I have little enthusiasm.  It's a rather
> >inefficient way of processing a lot of files anyway.
> 
> It's a nice syntax.  If it's grossly inefficient, we should improve the
> code; being built in to the shell it *ought* to be more efficient than
> execing cat, at least for small files.

It doesn't exec a cat, it does the reading/writing itself (in closemn).
I'm not sure how we could make this more efficient without using something
like mmap().

rc

Re: more problems w/ irix

[Zefram]

>> It's a nice syntax.  If it's grossly inefficient, we should improve the
>> code; being built in to the shell it *ought* to be more efficient than
>> execing cat, at least for small files.
>
>It doesn't exec a cat, it does the reading/writing itself (in closemn).

I'm aware of that.  I was responding to the statement that it is
inefficient, saying that "foo <a <b <c" should be more efficient than
"cat a b c | foo".  If it isn't, we should make it so.

>I'm not sure how we could make this more efficient without using something
>like mmap().

I think using mmap() would be a good move.  An madvise(), as used by
some cats, would also help performance.  We can do autoconf checks for
these functions, and would in any case need to have the code fall back
on a read/write loop in case mmap() is impossible.

-zefram

Re: more problems w/ irix

[Richard Coleman]

> >It doesn't exec a cat, it does the reading/writing itself (in closemn).
> 
> I'm aware of that.  I was responding to the statement that it is
> inefficient, saying that "foo <a <b <c" should be more efficient than
> "cat a b c | foo".  If it isn't, we should make it so.
> 
> >I'm not sure how we could make this more efficient without using something
> >like mmap().
> 
> I think using mmap() would be a good move.  An madvise(), as used by
> some cats, would also help performance.  We can do autoconf checks for
> these functions, and would in any case need to have the code fall back
> on a read/write loop in case mmap() is impossible.

It's been a while since I looked at the source for something like
GNU cat, but I remember it being quite a bit of code (relative to
what it does).  Also I believe it takes quite a bit of work to
write portable code that uses mmap().

I think of the multio as more of a convenience, rather than being
faster just because it is built into the shell.  For scripting
purposes (where speed is more important), you can just use
"cat a b c | whatever".

I think it would be sufficient to just output an error message
such as "max multio's is 16", when someone tries
"whatever < *".  They should use "cat * | whatever" instead.

We should probably mention this limit in the man pages.

rc

Re: more problems w/ irix

[Peter Stephenson]

Zefram wrote:
> It's a nice syntax.  If it's grossly inefficient, we should improve the
> code; being built in to the shell it *ought* to be more efficient than
> execing cat, at least for small files.

It's not hard to allow an indefinite number of files for input
(i.e. cat) if you record their names instead of opening them
immediately.  The downside is some rather unexpected behaviour: the
files are opened at random stages, not when the redirection line is
handled, and an error message is only produced when the file is opened
so that the command will already have been executed and all files up
to the bad one will already have been output.  (Of course, it would be
set up so that the first file in the list is opened straight away.)
Maybe this is too partial a solution to consider seriously, although
it does correspond to the behaviour of cat.  I've got it working but
for these reasons I deliberately haven't included it.

-- 
Peter Stephenson <pws@ifh.de>       Tel: +49 33762 77366
WWW:  http://www.ifh.de/~pws/       Fax: +49 33762 77413
Deutches Electronen-Synchrotron --- Institut fuer Hochenergiephysik Zeuthen
DESY-IfH, 15735 Zeuthen, Germany.