diff options
author | rse <rse@FreeBSD.org> | 2005-09-06 19:30:00 +0000 |
---|---|---|
committer | rse <rse@FreeBSD.org> | 2005-09-06 19:30:00 +0000 |
commit | fe548cd4faa84d32989fb24c872f832d854ad15d (patch) | |
tree | e4a8362d71de5834d0beea21b581957ecf576bb2 /bin/sh | |
parent | 4c165daa8dfedaee3ceb48cb97ff9fbabab36789 (diff) | |
download | FreeBSD-src-fe548cd4faa84d32989fb24c872f832d854ad15d.zip FreeBSD-src-fe548cd4faa84d32989fb24c872f832d854ad15d.tar.gz |
Various small code cleanups resulting from a code reviewing
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@
Diffstat (limited to 'bin/sh')
-rw-r--r-- | bin/sh/expand.c | 6 | ||||
-rw-r--r-- | bin/sh/miscbltin.c | 8 | ||||
-rw-r--r-- | bin/sh/trap.c | 2 |
3 files changed, 4 insertions, 12 deletions
diff --git a/bin/sh/expand.c b/bin/sh/expand.c index f087f54..e980589 100644 --- a/bin/sh/expand.c +++ b/bin/sh/expand.c @@ -893,7 +893,7 @@ numvar: } /* FALLTHROUGH */ case '*': - if (ifsset() != 0) + if (ifsset()) sep = ifsval()[0]; else sep = ' '; @@ -1022,8 +1022,7 @@ ifsbreakup(char *string, struct arglist *arglist) p++; } } while ((ifsp = ifsp->next) != NULL); - if (*start || (!ifsspc && start > string && - (nulonly || 1))) { + if (*start || (!ifsspc && start > string)) { sp = (struct strlist *)stalloc(sizeof *sp); sp->text = start; *arglist->lastp = sp; @@ -1211,7 +1210,6 @@ expmeta(char *enddir, char *name) scopy(dp->d_name, enddir); addfname(expdir); } else { - char *q; for (p = enddir, q = dp->d_name; (*p++ = *q++) != '\0';) continue; diff --git a/bin/sh/miscbltin.c b/bin/sh/miscbltin.c index cfd9e82..51ecee6 100644 --- a/bin/sh/miscbltin.c +++ b/bin/sh/miscbltin.c @@ -192,13 +192,7 @@ readcmd(int argc __unused, char **argv __unused) continue; } startword = 0; - 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) { STACKSTRNUL(p); setvar(*ap, stackblock(), 0); ap++; diff --git a/bin/sh/trap.c b/bin/sh/trap.c index ca327b2..5f12379 100644 --- a/bin/sh/trap.c +++ b/bin/sh/trap.c @@ -382,7 +382,7 @@ onsig(int signo) */ if (Tflag && trap[signo] != NULL && - ! trap[signo][0] == '\0' && + ! (trap[signo][0] == '\0') && ! (trap[signo][0] == ':' && trap[signo][1] == '\0')) breakwaitcmd = 1; |