termination with set -e if a command fails in a loop body inside a function
with an explicitely tested exit status, eg
f() {
for i in 1 2 3; do
false
done
}
f || true
Briefly reviewed by: cracauer
two cases of unwanted termination with set -e:
* if-commands containing several commands separated by semicolons, eg
if false; false; then [...]
* functions with an explicitely tested exit status that contain a failing
command which is not the last one, eg
f() {
false
false
}
f || true
PR: 77067, 85267
Briefly reviewed by: cracauer
and linting procedure:
1. Remove useless sub-expression:
- if (*start || (!ifsspc && start > string && (nulonly || 1))) {
+ if (*start || (!ifsspc && start > string)) {
The sub-expression "(nulonly || 1)" always evaluates to true and
according to CVS logs seems to be just a left-over from some
debugging and introduced by accident. Removing the sub-expression
doesn't change semantics and a code inspection showed that the
variable "nulonly" is also not necessary here in any way (and the
expression would require fixing instead of removing).
2. Remove dead code:
- if (backslash && c == '\\') {
- if (read(STDIN_FILENO, &c, 1) != 1) {
- status = 1;
- break;
- }
- STPUTC(c, p);
- } else if (ap[1] != NULL && strchr(ifs, c) != NULL) {
+ if (ap[1] != NULL && strchr(ifs, c) != NULL) {
Inspection of the control and data flow showed that variable
"backslash" is always false (0) when the "if"-expression is
evaluated, hence the whole block is effectively dead code.
Additionally, the skipping of characters after a backslash is already
performed correctly a few lines above, so this code is also not
needed at all. According to the CVS logs and the ASH 0.2 sources,
this code existed in this way already since its early days.
3. Cleanup Style:
- ! trap[signo][0] == '\0' &&
+ ! (trap[signo][0] == '\0') &&
The expression wants to ensure the trap is not assigned the empty
string. But the "!" operator has higher precedence than "==", so the
comparison should be put into parenthesis to form the intended way of
expression. Nevertheless the code was effectively not really broken
as both particular NUL comparisons are semantically equal, of course.
But the parenthesized version is a lot more intuitive.
4. Remove shadowing variable declaration:
- char *q;
The declaration of symbol "q" hides another identical declaration of
"q" in the same context. As the other "q" is already reused multiple
times and also can be reused again without negative side-effects,
just remove the shadowing declaration.
5. Just small cosmetics:
- if (ifsset() != 0)
+ if (ifsset())
The ifsset() macro is already coded by returning the boolean result
of a comparison operator, so no need to compare this boolean result
again against a numerical value. This also aligns the macros usage to
the remaining existing code.
Reviewed by: stefanf@
converting the stat() call to a lstat() call, which will cover the
situation. One can exercise this bug by referring a dangling link with
something like */the-link.
Approved by: re (scottl)
Submitted by: Simon 'corecode' Schubert [corecode fs ei tum de]
Obtained from: NetBSD via DragonFlyBSD (NetBSD rev. 1.51 and DragonFly
rev. 1.6)
MFC After: 3 days
promised by the Argument List Processing section introduction.
What follows the option in the options list is its long name,
not its argument (as is the case for the -c option). Also
sort references in the SEE ALSO section.
Approved by: re (blanket)
benefit of scripts start out as: #!/bin/sh -- # -*- perl -*-
With this fix in place, we can commit a change to kern/imgact_shell.c
so FreeBSD will process the `#!' line in shell-scripts in a more
standard fashion.
PR: 16393
Mentioned on: freebsd-arch
- Move the description of the ``-c string'' option closer to the option itself.
- Add an ENVIRONMENT section (1)
- Add more .Xr cross references to the SEE ALSO section.
Obtained from: NetBSD (1)
work as expected when they have a "shebang line" of:
#!/bin/sh -- # -*- perl -*- -p
This specific line is recommended in some perl documentation, and I think
I've seen similar lines in documentation for ruby and python. Those
write-ups expect `sh' to ignore everything after the '--' if the first
thing after the '--' is a '#'. See chapter 19, "The Command-Line Interface"
in 3rd edition of "Programming Perl", for some discussion of why perl
recommends using this line in some circumstances.
The above line does work on solaris, irix and aix (as three data points),
and it used to work on FreeBSD by means of a similar patch to execve().
However, that change to execve() effected *all* shells (which caused
other problems), and that processing was recently removed.
PR: 16393 (the original request to fix the same issue)
Reviewed by: freebsd-current (looking at a slightly different patch)
MFC after: 1 week
XXX from Tor: "The shell can also go into a similar loop if the child was
killed by signal 127, since the shell would believe the child to have
only stopped (WIFSTOPPED() macro returns nonzero value). Disallowing
signals 127 and 128 will fix that problem." See kern/19402 for details.
PR: bin/66242
Submitted by: tegge
Analysis and testcase by: demon
MFC after: 3 weeks
instead of just !, this allows one to more easily locate/understand
the section of the manpage in question.
Additional wording correction by: keramida
Reviewed by: keramida
considered an error according to the Open Group Base Specification.
PR: standards/45738
Submitted by: Matthias Andree <matthias.andree@web.de>
MFC after: 3 days
Only use return value from system call if system call succeeded.
Tested with `make world` and some of my own scripts.
This should be MFCed soon. While /bin/sh is hard to test the fix is
obviously correct and can be assumed not to break something else
(famous last words...).
Joe Marcus Clarke <marcus@FreeBSD.ORG>, subshells could lose a
non-zero exit status.
This commit is Joe's proposed patch. Thanks!
I verified that the problem Joe found is fixed and I ran a full world
with this patch.
I don't plan to ever commit language patches to /bin/sh again. It is
a minefield too big to navigate without a full-time committment, which
I am not willing to do on our /bin/sh.
Under normal circumstances I would recommend using NetBSD's sh which
has a lot of language fixes (like the ones what these patches were
about) but unfortunately they had implemented broken signal behaviour
for shellscript containing interactive programs. Similar issues apply
to pdksh which is OpenBSD's sh.
From my perspective bash2 is the only really working bourne sh out
there and that one is GPLed. Oh well.
should slightly reduce the number of system calls in critical portions of
the shell, and select a more efficient path through the fdalloc code.
Reviewed by: bde
sh -e behaviour was incorrect when && and || statements where used in
"if" clauses.
This is the patch submitted by MORI Kouji <mori@tri.asanuma.co.jp>.
It fixes the issue at hand, but sh fixes like this are super-hard to
verify that they don't break anything else. I ran some of my old test
cases and a few big GNU configure scripts that detected mistakes
before, with the previous sh, patched sh and bash. No differences in
behaviour found. MFC recommended after longer than usual time.
Compiles on i386 and sledge.
when grepping for JOBS. The recent style cleanup replaced the space with
a tab and broke job control detection. Little edits, disastrous consequences.
Submitted by: Peter Edwards <pmedwards@eircom.net>
X-MFC when: in about 5 weeks with the other sh arithmetic fixes.
- Removed dead declarations
- Made objects that should have been declared as static, static.
The changes use STATIC instead of static, following the existing
convention in the rest of the code.
Approved by: schweikh (mentor)
MFC after: 2 weeks
output buffer, don't insert them at all. This prevents a buffer
*underrun* when the substitution consists completely of newlines
(e.g. `echo`) and the byte before the source buffer to which p
points is a '\n', in which case more characters would be removed
from the output buffer than were inserted.
This fixes certain port builds on sparc64.
Approved by: re (scottl)
Reviewed by: des, tjr
Due to the use of signed vs. unsigned chars on our various platforms, one gets
"warning: comparison is always true due to limited range of data type"
from GCC 3.3.
mutually exclusive. The fact that the most recent one specified on the
command line is the one that takes effect is an implementation detail and
users should not rely on this.
The initial stack_block is staticly allocated and will be aligned
according to the alignment requirements of pointers, which does not
necessarily match the alignment enforced by ALIGN. To solve this a
more involved change is required: remove the static initial stack
and deal with an initial condition of not having a stack at all. This
change is therefore more risky than the previous ones, but unavoidable
(other than not using the platform default alignment).
Discussed with: tjr
Approved and reviewed by: tjr
Tested on: alpha, i386, ia64 and sparc64
The problem with the previous attempt, as noticed by Marcel, was that
stacknxt was being aligned to a pointer boundary instead of an
ALIGNBYTES + 1 boundary, which broke sparc64.
using the alignment from sys/param.h (16) instead of the alignment
from machdep.h (8) tickled a nasty bug in the memory allocator that I
haven't been able to track down yet.
are later stripped with rmescapes() in expandarg(). If the filename has
already been unescaped, doing it again in rmescapes() can walk off the
end of the string, leading to memory corruption and eventually SIGSEGV.
Noticed by: kris
commands. Commands like "if then ... fi" and "while do ... done" are no
longer accepted. Bodies of compound commands are still allowed to be
empty, because even though POSIX does not allow them, most shells do.
pointers. This fixes two format warnings on 64 bits
archs which are fatal now that WFORMAT=0 has been removed.
It doesn't fully fix the sh(1) build on 64 bits platforms
though, there is still some quad_t issues that need to be
fixed.
Tested on: i386, sparc64
adding history and vi/emacs-style line editing to the shell itself.
Atty was a user-mode terminal emulator (like screen and window) that did
line editing and history.
forkshell() after it has been freed. This caused mysterious behaviour
when anything but the first command in a pipeline tried to access the
terminal when the `junk' malloc() option was enabled (which is the default).
itself does that if you set EL_SIGNAL. Instead, set a flag and check it
before calling el_gets(). This is safer, but slower to respond to changes.
Pointed out by: mp
shell function and alias lookup. The -p option has been implemented, the
UPE -v and -V options have not. The old `command' command has been renamed
to `builtin'.
The pgrp member of struct job was declared as a short and could not store
every possible process group ID value, the rest of them were benign because
pid_t happens to be an int.
2. Instead, open /dev/tty. This problem stopped commands in subshells from
being executed correctly if standard error was redirected.
PR: 36671
Obtained from: NetBSD (but simplified)
reality (and POSIX): current directory isn't searched unless CDPATH has
a "." element or is unset.
PR: 38442
Submitted by: oleg dashevskii <be9@be9.ru>
MFC after: 1 week
keep a linked list of the jobs, most recently used first. This is required
to support the idea of `previous job', and to allow the jobs fg and bg
default to be correct according to POSIX.
to fail when the logical current directory no longer exists. Allow changes
to absolute paths when logical cwd is invalid, fall back to physical cd
if logical cd fails.
o Old-style K&R declarations have been converted to new C89 style
o register has been removed
o prototype for main() has been removed (gcc3 makes it an error)
o int main(int argc, char *argv[]) is the preferred main definition.
o Attempt to not break style(9) conformance for declarations more than
they already are.
o Change
int
foo() {
...
to
int
foo(void)
{
...
used so often that it's worth keeping it as a builtin.
Now that all the printf invocations from within the system startup
scripts, we can safely remove it.
Urged by: sheldonh :)
No MFC is planned so far because it may break compatibility and
violate POLA.
binary size increase is 3,784 bytes (about 0.6%).
I don't drop the printf builtin while I'm here because some /etc/rc.*
scripts seem to use it before mounting /usr where printf(1) resides.
Reviewed by: arch (sheldonh)
Inspired by: NetBSD, ksh
Clued by: ume (on how the printf builtin is used)
setvar() and passed to setvareq(). When the VTEXTFIXED flag is set,
that copy is never freed, causing a memory leak.
PR: 31533
Submitted by: maxim@macomnet.ru
value CTLARI since this might break expansion of arithmetic expressions.
Don't access memory below start of stackblock.
Problem analyzed by hunt@iprg.nokia.com, slightly different patch applied.
PR: 24443
Submitted by: hunt@iprg.nokia.com
Avoid using parenthesis enclosure macros (.Pq and .Po/.Pc) with plain text.
Not only this slows down the mdoc(7) processing significantly, but it also
has an undesired (in this case) effect of disabling hyphenation within the
entire enclosed block.
errexit (-e) processing. This solves a problem where 'make clean' would
fail with an unspecified error in certain automake-generated makefiles.
Reviewed by: no objections from -hackers...
MFC after: 2 weeks
When a child is receiving SIGSTOP, eval continues with the next
command. While that is correct for the interactive case (Control-Z
and you get the prompt back), it is wrong for a shellscript, which
just continues with the next command, never again waiting for the
stopped child. Noted when childs from cronjobs were stopped, just to
make more processes (by wosch).
The fix is not to return from a job wait when the wait returned for a
stopped child while in non-interactive mode. This bahaviour seems to
be what bash2 and ksh implement. I tested for correct behaviour for
finnaly killing the child with and without forgrounding it first.
When not foregrouding before killing, the shell continues with the
script, which is what the other shells do as well.
Reviewed by: Silence on -current
Serious fix still needed, see discussion on -current
(Subject: /bin/sh dumps core with here-document of 8bit text)
Problem in this code originally spotted by
Jun Kuriyama <kuriyama@FreeBSD.org>
growstackblock() sometimes relocates a stack_block considered empty
without properly relocating stack marks referencing that block.
The first call to popstackmark() with the unrelocated stack mark
as argument then causes sh to abort.
Relocating the relevant stack marks seems to solve this problem.
The patch changes the semantics of popstackmark() somewhat. It can
only be called once after a call to setstackmark(), thus cmdloop() in
main.c needs an extra call to setstackmark().
PR: bin/19983
Submitted by: Tor.Egge@fast.no
Reviewed by: Gerald Pfeifer <pfeifer@dbai.tuwien.ac.at>
case), so that it doesn't clash with the ncurses function of the same
name when linking statically with -ltermcap.
The linker only complains when -static is used, and it is not clear
whether this is a bug.
PR: bin/18104
Submitted by: Anatoly Vorobey <mellon@pobox.com>
spaces reserved by the header files it includes.
mkinit.c still produces C code with redundant declarations, although
they are more harmless since they automatically derived from the right
places.
in that revision as well as things I broke in that revision. A note-
worthy instance of the latter case was the inversion of -E and -V in the
subsection on Commandline Editing.
than two processes (got that? :-), the stdin fd of the middle
processes that has just been set up was accidetially closed. Don't do
this.
PR: bin/14527
commit and those which cause ugly nroff output have been fixed, since
the purpose of the style guideline which they contravene is to reduce
the sizes of deltas.
Reported by: bde
* Consistently misspell built-in as builtin.
* Add a builtin(1) manpage and create builtin(1) MLINKS for all shell
builtin commands for which no standalone utility exists. These MLINKS
replace those that were created for csh(1).
* Add appropriate xrefs for builtin(1) to the csh(1) and sh(1) manpages,
as well as to the manpages of standalone utilities which are supported
as shell builtin commands in at least one of the shells. In such
manpages, explain that similar functionality may be provided as a
shell builtin command.
* Improve sh(1)'s description of the cd builtin command. Csh(1) already
describes it adequately. Replace the cd(1) manpage with a builtin(1)
MLINKS link.
* Clean up some mdoc problems: use Xr instead of literal "foo(n)"; use
Ic instead of Xr for shell builtin commands.
* Undo English contractions.
Reviewed by: mpp, rgrimes
Fix grammar and spelling nits.
Use .Dq and .Qq where appropriate.
Divorce trailing punctuation from quoted elements.
Use .Dq instead of .Xr for builtins.
Remove trailing whitespace and blank lines.
PR: 13340
statement if blocks[*] when the else could be ambiguous, not defaulting
to int type and removal of some unused variables.
[*] This is explicitly allowed by style(9) when the single statement
spans more than one line.
Reviewed by: obrien, chuckr
representation of the expression is quoted. Take care of this when
doing pattern matching in conjunction with trimming.
#!/bin/sh
c=d:e; echo "${c%:e}"
PR: NetBSD PR#7231
Noticed by: Havard Eidnes <Havard.Eidnes@runit.sintef.no>
make /etc/rc interruptible in cases when programs hang with blocked
signals) isn't standard enough.
It is now switched off by default and a new switch -T enables it.
You should update /etc/rc to the version I'm about to commit in a few
minutes to keep it interruptible.
Don't output double-quotes inside variable expansion/arithmetic
expansion region in here-documents. When leaving the arithmetic
expansion syntax mode, adjust the dblquote flag according to
previous syntax, in order to avoid splitting of quoted variables.
foreground child is running. Formerly, traps were exceuted after the
next child exit.
The enables the user to put a breaking wrapper around a blocking
application:
(trap 'echo trap ; exit 1' 2; ./pestyblocker; echo -n)
The "echo -n" after the child call is needed to prevent sh from
optimizing the trap-executing shell away. I'm working on this.
multiple times when performing nested variable expansion, and
preserve some quoting information in order to avoid removing
apparently empty expansion result.
i.e. this makes emacs usable from system(3). Programs called from
shellscripts are now required to exit with proper signal status. That
means, they have to kill themself. Exiting with faked numerical exit
code is not sufficient.
Exit with proper signal status if script exits on signal.
Make the wait builtin interruptable, both with and without traps set.
Use volatile sig_atomic_t where (and only where) appropriate.
(Almost) fix printing of newlines on SIGINT.
Make traps setable from trap handlers. This is needed for shellscripts
that catch SIGINT for cleanup work but intend to exit on it, hance
have to kill themself from a trap handler. I.e. mkdep.
While I'm at it, make it -Wall clean. -Wall is not enabled in
Makefile, since vararg warnx() macro calls in usr.bin/printf/printf.c
are not -Wall-able.
PR: 1206
Obtained from: Basic SIGINT fix from Bruce Evans
Removed explicit dependencies of foo.o on foo.c. These were mainly
placeholders for comments about missing dependencies of tools objects
on headers. This problem needs to be handled more generally.
urgent need is when you run sh around a program that intentionally
uses SIGQUIT/SIGINT for asynchronous events, i.e. $EDITOR started from
system(2), like many mailers do. This fixes PR bin/1206 and possibly
bin/4241.
The solution committed has been tested for a large number of possible
cases (see recent discussion on cvs-committers). I completed a make
world, made sure 'make world' is interruptable and used the changed
/bin/sh as a login shell all day, including job control and using
SIGQUIT-catching programs (to write this message :-).
PR: bin/1206
Reviewed by: discussion on cvs-commiters
cast value that was always ignored. Rev.1.9 of trap.c made this
more bogus by returning a semantically different value after calling
siginterrupt(). Avoid these problems by not returning a value.
trap 'echo xxx' 1 2 3 15
read x
is not interrupted by ^C (due to restartable read syscall) and must be
interrupted per POSIX
Worse case:
read -t 5 x
hangs forever after ^C pressed (supposed to timeout after 5 secs)
Fixed by adding siginterrupt(signo, 1) after catch handler installed
2) Do not reinstall sighandler immediately after it is called,
BSD do it for us
'read' command to return an error if the user fails to supply any
input withink a given time period. The behaviour of this option is
similar to that of the like-named option in ksh93.
Reviewed by: joerg
This will make a number of things easier in the future, as well as (finally!)
avoiding the Id-smashing problem which has plagued developers for so long.
Boy, I'm glad we're not using sup anymore. This update would have been
insane otherwise.
while remaining (becoming :) compatible with other popular shells.
Specifically these changes include:
1) Implement 'trap -l' to get a list of valid signals names. This
is useful if you wanted to do something like reset all signal
handlers to there defaults values, in which case something like
this will do the trick.
trap `trap -l`
2) Reformat the output of 'trap' so it can be saved and later eval'd
to restore the saved settings.
3) Allow the use of signal names as well as signal numbers.
4) Fix trap handling of SIGCHLD so that commands like the following
(albeit, contrived) won't cause sh(1) to recurse ad infinitum.
trap uname 0 20
5) Make variables static that are used only in trap.c.
6) Minor 'style(9) police' mods.
now handles the getpwd() init problem the same way as bash
and ksh do. Also while I was in here, I cleaned up the format
a little, removed some unnnecessary #if SYMLINKS cruft, and
changed the pwd builtin to use getcwd(3) as Joerg suggested.
getopts should now work as expected. This fix was in the NetBSD
code that I was merging from but missed getting into FreeBSD's
version because of 'drain bamage' on my part.
Submitted by: NetBSD, joerg
This patch causes too many side effects, one of which bites hard is
when interrupting a 'make fetch' in the ports tree (PR#1990).
This whole area is a real can of worms....
This most definately should go into 2.2
Reviewed by: steve, bde
so that simple regresssion tests based on `cmp' work. mkdep still
doesn't work right for these tools. They should probably be in
separate directories.
Sorted dependencies.
face of aliases. Note, bash doesn't do aliases while running scripts, but
"real" ksh does..
Also:
Reduce redundant .Nm macros in (unused) bltin/echo.1
nuke error2, it's hardly used.
More -Wall cleanups
dont do certain history operations if NO_HISTORY defined
handle quad_t's from resource limits
Submitted by: Steve Price <sprice@hiwaay.net> (minor tweaks by me)
- don't put \n on error() calls, error adds it already.
- don't prepend "ulimit" on error() calls in miscbltin.c.
- getopt typo on ulimit -p -> -u conversion
- get/setrlimit() calls were not being error checked
ulimit formatting cleanup from me, use same wording as bash on Bruce's
suggestion. Add ulimit arg to output on Joerg's suggestion.
merge of parallel duplicate work by Steve Price and myself. :-]
There are some changes to the build that are my fault... mkinit.c was
trying (poorly) to duplicate some of the work that make(1) is designed to
do. The Makefile hackery is my fault too, the depend list was incomplete
because of some explicit OBJS+= entries, so mkdep wasn't picking up their
source file #includes.
This closes a pile of /bin/sh PR's, but not all of them..
Submitted by: Steve Price <steve@bonsai.hiwaay.net>, peter
Requested by: joerg
(Note, this is mostly going to be conflicts, which is expected. Our entire
sh source has a mainline, so this should not change anything except for
a few new files appearing. I dont think they are a problem)
didn't correctly start background jobs anymore. Strange that nobody
was complaining...
Add a dummy target for `builtins' in the Makefile, to prevent it
from attempting to build this file by compiling builtins.c. :-/
traditional behaviour, and it violates Posix.2.
Fixes PR # bin/880: /bin/sh incorrectly parse...
Fixes also an earlier problem report about the shell not evaluating
loops correctly. (Not files via GNATS.)
Submitted by: nnd@itfs.nsk.su (Nickolay N. Dudorov)
This means that a script containing:
echo 1
set -v
echo 2
will now produce output, like it does on SYSV machines and other 'proper'
/bin/sh implementations..
This is done by a slight restructure of the input processor allowing it to
read chunks from the file at a time, but process the data by line from the
chunk.
Obtained from: Christos Zoulas for NetBSD. <christos@deshaw.com>
o fix brokeness for 1>&5 redirection, where `5' was an invalid file
descriptor, but no error message has been generated
o fix brokeness for redirect to/from myself case
command and badly needed in sh(1) for everybody who wants to modify
the system-wide limits from inside /etc/rc.
The options are similar to other system's implemantations of this
command, with the FreeBSD additions for -m (memoryuse) and -p (max
processes) that are not available on other systems.
