"getopts" bugs and bad interactions with "shift"

"getopts" bugs and bad interactions with "shift"

[Bart Schaefer]

Consider:

function oops() {
    while getopts :m: f
    do
	echo $OPTIND is $f with $OPTARG
	shift $((OPTIND - 1))
    done
}

3.0.7 gives:

zagzig% oops -m 0664 something
3 is m with 0664
zagzig% 

3.1.6-pws-9 gives:

zagzig% oops -m 0664 something 
2 is m with 0664
2 is ? with t
zagzig% 

There are two problems here: (1) OPTIND is different. (2) Parsing didn't
stop soon enough; it should have returned nonzero once -m was shifted off.
You can see the latter by changing the optstring to :m:t:, which gives

2 is m with 0664
2 is t with hing

I originally thought both had something to do with 7765,  but it behaves
like this at least as far back as 3.1.6-pws-4 (I didn't try any older).

I believe the following has something to do with (2), though this can't be
the entire issue, because I should be able to "fool" 3.0.7 by putting '-'
at the beginnings of selected words; yet I find I can't.

In 3.0 (which lacks the getopts rewrite from 3.1.2-zefram-3), this test
is used:

    if ((*str != '+' && *str != '-') || optcind >= lenstr ||
	(lenstr == 2 && str[0] == '-' && str[1] == '-')) {
	/* current argument doesn't contain options, or optcind is impossibly
	large */
	if (*str == '+' || *str == '-')
	    zoptind++;
	optcind = 0;
	return 1;
    }

But 3.1.x for x > 2 uses:

    if(optcind >= lenstr) {
	optcind = 0;
	if(!args[zoptind++])
	    return 1;
	str = unmetafy(dupstring(args[zoptind - 1]), &lenstr);
    }
    if(!optcind) {
	if(lenstr < 2 || (*str != '-' && *str != '+'))
	    return 1;
	if(lenstr == 2 && str[0] == '-' && str[1] == '-') {
	    zoptind++;
	    return 1;
	}
	optcind++;
    }

Note that the test of (*str != '-' && *str != '+') isn't done unless the
index into the string is zero, but the index isn't reset to zero unless
it's at or past the end of the string to begin with.

(At the very least, optcind should be getting reset to 0 whenever an
explicit assignment to OPTIND is done.)

However, this brings up the question of whether "getopts" and "shift" are
even -intended- to play nicely together.  I thought briefly about putting
an "optcind = 0;" into bin_shift(), but that doesn't cover this case:

    while getopts :m: f
    do
	argv[0,OPTIND-1]=()
    done

Nor this:

    array=($*)
    while getopts :m: f $array
    do
	array[OPTIND]=()
    done

Finally, there's the question of exactly what OPTIND is supposed to be.
The documentation for both versions says:

OPTIND <S>
     The index of the last option argument processed by the getopts
     command.

But when you assign to OPTIND, you assign the index of the *next* option
to be processed by the getopts command -- and the 3.0.7 behavior is to
leave OPTIND set to that *next* index.  3.1.6, on the other hand, uses
(optcind >= lenstr) to pre-increment zoptind, which leads to this third
bug when the presumed option argument is the empty string (-pws-9 again):

zagzig% oops "" -m 0664 something 
3 is m with 0664
zagzig% 

Question:  Is "getopts" based on some standard or emulated behavior of
some other shell, or is it strictly a zsh thing?  If the latter, I think
we should change the documentation and the 3.1.6 behavior of OPTIND to
match the 3.0.7 behavior, which fixes (1) and should make it much easier
to fix (2) if in fact we decide that's supposed to work at all.

I think the third bug will disappear as a side-effect of fixing OPTIND.

Comments?

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: "getopts" bugs and bad interactions with "shift"

[Adam Spiers]

Bart Schaefer (schaefer@candle.brasslantern.com) wrote:
> Question:  Is "getopts" based on some standard or emulated behavior of
> some other shell, or is it strictly a zsh thing?

getopt is part of GNU libc, and according to the info pages:

  `getopt' is more standard [than argp_parse] (the short-option only
  version of it is a part of the POSIX standard),

Virtually all GNU utilities use it.  Also, Perl has the
Getopt::{Std,Long} modules which emulates it precisely, AFAIK.  A `man
3 getopt' on my RedHat box confirms what I suspected, which is that
there is only one optstring parameter.  It also mumbles stuff about
POSIX being broken ...

Re: "getopts" bugs and bad interactions with "shift"

[Zefram]

Bart Schaefer wrote:
>Question:  Is "getopts" based on some standard or emulated behavior of
>some other shell, or is it strictly a zsh thing?

It's in POSIX.2.  My rewrite was an attempt to directly implement
the text of the standard.  (I no longer have access to the standard,
unfortunately.)  As you can tell from the code, the standard definition
is pretty unpleasant.  There are places where it's not clear what state
is stored in parameters and what is supposed to be hidden, and the text
doesn't address the issue of multiple consecutive uses of getopts at all.
I wouldn't be surprised if there are actually bugs in the standard.

-zefram

Re: "getopts" bugs and bad interactions with "shift"

[Bart Schaefer]

On Nov 23,  6:22pm, Adam Spiers wrote:
> Subject: Re: "getopts" bugs and bad interactions with "shift"
> Bart Schaefer (schaefer@candle.brasslantern.com) wrote:
> > Question:  Is "getopts" based on some standard or emulated behavior of
> > some other shell, or is it strictly a zsh thing?
> 
> getopt is part of GNU libc [...]

In case it wasn't obvious from Zefram's post, "getopts" != "getopt".

The GNU "getopt" command (short-args-only form) is very roughly equivalent
to

    function getopt () {
	optstr=":$1"; shift
	got=()
	ret=0
	while getopts "$optstr" opt
        do
	    if [[ $opt == [?] ]]
	    then
		echo getopt: invalid option -- "$OPTARG" 1>&2
		ret=1
	    else
		got=("$got[@]" "-$opt" ${OPTARG:+"$OPTARG"})
	    fi
	done
	shift $OPTIND	# $((OPTIND-1)) if 3.0.7 or before
	echo "" "$got[@]" -- "$@"
	return $ret
    }

zsh 3.0.7 hogs memory

[James Kirkpatrick]

We've run across an interesting problem where zsh 3.0.7 will grow in size,
consuming memory until the system paralyzes itself with swapping, then
suddenly break itself free.

Environment: zsh 3.0.7, Sun Enterprise 5000 with 2 Gig memory and 6
processors, Solaris 2.5.1.

It usually seems to happen to people running Pine (4.10).  My guess at
this point is that they dial in, start pine, and then either disconnect or
become disconnected.  This has often caused problems with programs that
try to handle exceptions and fail in some cases (I've seen this a lot
starting with SAS on Cyber mainframes in 1980).

I tried running a script that sleeps for 1 second then asks "how long have
I been sleeping?".  If the answer is over 2 seconds it does a
"ps -eo pmem,user,pcpu,rss,vsz,fname | sort | tail"
to a log file.  Some of the entries include:

  %MEM     USER %CPU  RSS  VSZ COMMAND
  12.3    hbear  3.1 252832 599944 zsh
  71.3  skuyper  8.1 1466520 1941360 zsh
  86.1    renee 13.2 1772384 1941352 zsh

86% of 2 Gig is 1.72 Gig, a ridiculous amount for a shell to consume, so
clearly there is a problem.

One other clue:  This only started happening when I added the following to
/etc/zshenv:

  export HISTSIZE=200
  export HISTFILE=~/.zsh_history
  export SAVEHIST=200

Once I commented it out, the problem went away.  SO, I'm suspicious of
problems in exception handling (disconnect) with history files turned on.
Of course I might be wrong about the exceptions issue.

Any suggestions/fixes?

Jim

Re: zsh 3.0.7 hogs memory

[Bart Schaefer]

On Nov 23,  3:17pm, James Kirkpatrick wrote:
} Subject: zsh 3.0.7 hogs memory
}
} It usually seems to happen to people running Pine (4.10).  My guess at
} this point is that they dial in, start pine, and then either disconnect or
} become disconnected.
} 
} One other clue:  This only started happening when I added the following to
} /etc/zshenv:
} 
}   export HISTSIZE=200
}   export HISTFILE=~/.zsh_history
}   export SAVEHIST=200
} 
} Once I commented it out, the problem went away.  SO, I'm suspicious of
} problems in exception handling (disconnect) with history files turned on.

(I'm going to assume that by "exceptions" you mean "signals".)

The relevant bits of code would be:

	signals.c: 464: handler()
	builtin.c: 4761: zexit()
	hist.c: 1502-1584: savehistfile()
and maybe
	signals.c: 543: killrunjobs()

The only thing I could guess that might be happening is:

handler(), invoked on SIGHUP, calls zexit(), which calls killrunjobs();
killrunjobs() finds a job with group leader the current shell, and sends
it a SIGHUP; this invokes handler() again, and we loop.

Now, it *should* be the case that during the handling of a SIGHUP, any
additional SIGHUP is blocked.  The only possible connection I can come
up with between this and savehistfile() is if the stdio operations that
savehistfile() is using are somehow unblocking the signal, which seems
*very* unlikely.

You could attach dbx or gdb to one of the runaway processes (the smallest
one) and get a look at the stack.  Or (less helpful) you could try trapping
the HUP signal by adding this to /etc/zshenv as well:

	TRAPHUP() { exit 1 }

This will invoke zexit() in a slightly different way ("from_signal" will
be false) and should avoid the SIGHUP loop I described, if in fact that
is the problem.

Or you could assume that's the problem and try this patch:

Index: Src/signals.c
===================================================================
@@ -540,7 +540,8 @@
         if ((from_signal || i != thisjob) && (jobtab[i].stat & STAT_LOCKED) &&
             !(jobtab[i].stat & STAT_NOPRINT) &&
             !(jobtab[i].stat & STAT_STOPPED)) {
-            if (killpg(jobtab[i].gleader, SIGHUP) != -1)
+            if (jobtab[i].gleader != getpid() &&
+		killpg(jobtab[i].gleader, SIGHUP) != -1)
                 killed++;
         }
     if (killed)

I'll be a bit concerned about other signal-handling issues if that really
does fix it, though.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: zsh 3.0.7 hogs memory

[James Kirkpatrick]

Thanks for the reply, I'll mull over the options and give one a try.  If
this is an issue with 3.0.7 I assume it's also in 3.1.x.

And yes, "signals" (wrong OS/hardware terminology :-).

Jim

On Wed, 24 Nov 1999, Bart Schaefer wrote:

> On Nov 23,  3:17pm, James Kirkpatrick wrote:
> } Subject: zsh 3.0.7 hogs memory
> }
> } It usually seems to happen to people running Pine (4.10).  My guess at
> } this point is that they dial in, start pine, and then either disconnect or
> } become disconnected.
> } 
> } One other clue:  This only started happening when I added the following to
> } /etc/zshenv:
> } 
> }   export HISTSIZE=200
> }   export HISTFILE=~/.zsh_history
> }   export SAVEHIST=200
> } 
> } Once I commented it out, the problem went away.  SO, I'm suspicious of
> } problems in exception handling (disconnect) with history files turned on.
> 
> (I'm going to assume that by "exceptions" you mean "signals".)
> 
> The relevant bits of code would be:
> 
> 	signals.c: 464: handler()
> 	builtin.c: 4761: zexit()
> 	hist.c: 1502-1584: savehistfile()
> and maybe
> 	signals.c: 543: killrunjobs()
> 
> The only thing I could guess that might be happening is:
> 
> handler(), invoked on SIGHUP, calls zexit(), which calls killrunjobs();
> killrunjobs() finds a job with group leader the current shell, and sends
> it a SIGHUP; this invokes handler() again, and we loop.
> 
> Now, it *should* be the case that during the handling of a SIGHUP, any
> additional SIGHUP is blocked.  The only possible connection I can come
> up with between this and savehistfile() is if the stdio operations that
> savehistfile() is using are somehow unblocking the signal, which seems
> *very* unlikely.
> 
> You could attach dbx or gdb to one of the runaway processes (the smallest
> one) and get a look at the stack.  Or (less helpful) you could try trapping
> the HUP signal by adding this to /etc/zshenv as well:
> 
> 	TRAPHUP() { exit 1 }
> 
> This will invoke zexit() in a slightly different way ("from_signal" will
> be false) and should avoid the SIGHUP loop I described, if in fact that
> is the problem.
> 
> Or you could assume that's the problem and try this patch:
> 
> Index: Src/signals.c
> ===================================================================
> @@ -540,7 +540,8 @@
>          if ((from_signal || i != thisjob) && (jobtab[i].stat & STAT_LOCKED) &&
>              !(jobtab[i].stat & STAT_NOPRINT) &&
>              !(jobtab[i].stat & STAT_STOPPED)) {
> -            if (killpg(jobtab[i].gleader, SIGHUP) != -1)
> +            if (jobtab[i].gleader != getpid() &&
> +		killpg(jobtab[i].gleader, SIGHUP) != -1)
>                  killed++;
>          }
>      if (killed)
> 
> I'll be a bit concerned about other signal-handling issues if that really
> does fix it, though.
> 
> -- 
> Bart Schaefer                                 Brass Lantern Enterprises
> http://www.well.com/user/barts              http://www.brasslantern.com

Re: zsh 3.0.7 hogs memory

[James Kirkpatrick]

Bart,

Of the three suggestions (attach dbx, add a TRAPHUP, recompile with patch)
I tried the TRAPHUP and that does indeed seem to solve the problem.
However, it seems more of an experiment than a true fix.

Does this give you enough to work towards a proper fix, or should I do
more to characterize the problem (e.g. try one of the other two
suggestions)?

Jim

On Wed, 24 Nov 1999, Bart Schaefer wrote:

> On Nov 23,  3:17pm, James Kirkpatrick wrote:
> } Subject: zsh 3.0.7 hogs memory
> }
> } It usually seems to happen to people running Pine (4.10).  My guess at
> } this point is that they dial in, start pine, and then either disconnect or
> } become disconnected.
> } 
> } One other clue:  This only started happening when I added the following to
> } /etc/zshenv:
> } 
> }   export HISTSIZE=200
> }   export HISTFILE=~/.zsh_history
> }   export SAVEHIST=200
> } 
> } Once I commented it out, the problem went away.  SO, I'm suspicious of
> } problems in exception handling (disconnect) with history files turned on.
> 
> (I'm going to assume that by "exceptions" you mean "signals".)
> 
> The relevant bits of code would be:
> 
> 	signals.c: 464: handler()
> 	builtin.c: 4761: zexit()
> 	hist.c: 1502-1584: savehistfile()
> and maybe
> 	signals.c: 543: killrunjobs()
> 
> The only thing I could guess that might be happening is:
> 
> handler(), invoked on SIGHUP, calls zexit(), which calls killrunjobs();
> killrunjobs() finds a job with group leader the current shell, and sends
> it a SIGHUP; this invokes handler() again, and we loop.
> 
> Now, it *should* be the case that during the handling of a SIGHUP, any
> additional SIGHUP is blocked.  The only possible connection I can come
> up with between this and savehistfile() is if the stdio operations that
> savehistfile() is using are somehow unblocking the signal, which seems
> *very* unlikely.
> 
> You could attach dbx or gdb to one of the runaway processes (the smallest
> one) and get a look at the stack.  Or (less helpful) you could try trapping
> the HUP signal by adding this to /etc/zshenv as well:
> 
> 	TRAPHUP() { exit 1 }
> 
> This will invoke zexit() in a slightly different way ("from_signal" will
> be false) and should avoid the SIGHUP loop I described, if in fact that
> is the problem.
> 
> Or you could assume that's the problem and try this patch:
> 
> Index: Src/signals.c
> ===================================================================
> @@ -540,7 +540,8 @@
>          if ((from_signal || i != thisjob) && (jobtab[i].stat & STAT_LOCKED) &&
>              !(jobtab[i].stat & STAT_NOPRINT) &&
>              !(jobtab[i].stat & STAT_STOPPED)) {
> -            if (killpg(jobtab[i].gleader, SIGHUP) != -1)
> +            if (jobtab[i].gleader != getpid() &&
> +		killpg(jobtab[i].gleader, SIGHUP) != -1)
>                  killed++;
>          }
>      if (killed)
> 
> I'll be a bit concerned about other signal-handling issues if that really
> does fix it, though.
> 
> -- 
> Bart Schaefer                                 Brass Lantern Enterprises
> http://www.well.com/user/barts              http://www.brasslantern.com

Re: zsh 3.0.7 hogs memory

[Bart Schaefer]

On Nov 30,  8:42am, James Kirkpatrick wrote:
} Subject: Re: zsh 3.0.7 hogs memory
}
} I tried the TRAPHUP and that does indeed seem to solve the problem.
} However, it seems more of an experiment than a true fix.

Yes.

} Does this give you enough to work towards a proper fix, or should I do
} more to characterize the problem (e.g. try one of the other two
} suggestions)?

Please try applying the patch and recompiling.  (And then be sure to remove
the TRAPHUP before you test it.)  The TRAPHUP experiment confirms that it's
a signal-handling issue rather than simply EOF-on-stdin, but it doesn't
entirely confirm my hypothesis about the SIGHUP loop.

Another thing that I just thought of:  It might be an interaction between
SIGHUP and SIGCHLD handlers; possibly saving of the histfile delays zsh's
exit just enough for the signal from the exiting Pine to arrive, or to
arrive in a different part of the shutdown.  (Anybody else have a guess?)

} > Index: Src/signals.c
} > ===================================================================
} > @@ -540,7 +540,8 @@
} >          if ((from_signal || i != thisjob) && (jobtab[i].stat & STAT_LOCKED) &&
} >              !(jobtab[i].stat & STAT_NOPRINT) &&
} >              !(jobtab[i].stat & STAT_STOPPED)) {
} > -            if (killpg(jobtab[i].gleader, SIGHUP) != -1)
} > +            if (jobtab[i].gleader != getpid() &&
} > +		killpg(jobtab[i].gleader, SIGHUP) != -1)
} >                  killed++;
} >          }
} >      if (killed)
} > 
} > I'll be a bit concerned about other signal-handling issues if that really
} > does fix it, though.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: "getopts" bugs and bad interactions with "shift"

[Bart Schaefer]

On Nov 24, 12:50pm, Sven Wischnowsky wrote:
} Subject: Re: "getopts" bugs and bad interactions with "shift"
}
} Bart Schaefer wrote:
} 
} > ...
} > 	shift $OPTIND	# $((OPTIND-1)) if 3.0.7 or before
} > ...
} > 	return $ret
} 
} I think, when *you* do this kind of stuff, you have a good reason, so:
} why the `$'s (and even `$((...))')? Compatibility with other shells?

Compatibility with my brain, is more like it.

When re-reading a script, I like being able to tell, without having to
think about the context, whether a string is just a string, or is the
name of a parameter.  I've never particularly liked the feature that
you can omit the $ in math context, though it almost makes sense in the
case of $[foo] and $((foo)) to not need $[$foo].  ("Almost" because of
this:

zagzig[21] setopt nounset
zagzig[22] echo $foo   
zsh: foo: parameter not set
zagzig[23] echo $[foo]
0

I hate not being able to use nounset to detect typographical errors in
parameter names.)

To a lesser extent, it's defensive programming in the sense that if I
don't get used to leaving off the $ in zsh scripts then I won't forget
to use it in plain sh scripts.

Finally, it means I don't have to remember where zsh uses math context
and where it doesn't; and the less I have to remember, the better.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: "getopts" bugs and bad interactions with "shift"

[Sven Wischnowsky]

Don't have any real opinion on this getopts business, just want to ask 
a question...

Bart Schaefer wrote:

> ...
> 	shift $OPTIND	# $((OPTIND-1)) if 3.0.7 or before
> ...
> 	return $ret

I think, when *you* do this kind of stuff, you have a good reason, so:
why the `$'s (and even `$((...))')? Compatibility with other shells?

Bye
 Sven

P.S.: They both matheval() their arguments...

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de