Page MenuHomeFreeBSD

sys/conf/newvers.sh: small fixes
Needs ReviewPublic

Authored by thebugfixers_pm.me on Fri, Jun 26, 12:45 PM.

Details

Reviewers
None
Group Reviewers
Src Committers
Contributor Reviews (src)
Summary

Main changes:

  • Invalid flags are not handled. Added a *) case.
  • In the unlikely event cd fails, treat as fatal (exit 1)
  • Prefer [ p ] && [ q ] as [ p -a q ] is not well defined, c.f. shellcheck SC2166 and its upstream reference to The Open Group Base Specifications.
  • Added shellcheck disable=SC3043 to maintain local instead of strict POSIX adherance
  • Added shellcheck disable=SC3028 to silence ${HOSTNAME:-$(hostname)} as the present parameter expansion safely handles the case where it's unset and we are not targetting strict POSIX adherance.

Also:

  • Use $(...) notation instead of legacy backticks
  • Double quote to prevent globbing and word splitting

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sys/conf/newvers.sh
163

So I'm inclined to reject this. This is a de-factor modern standard and all modern shells have local. The project does prefer things to be portable to POSIX, but the safety from this extension outweighs that ideal.

199

This should be documented in the commit message.

273

We use this construct in several places. How is it not well defined for this use?

299

ditto

sys/conf/newvers.sh
163

Fair. Noted. Happy to update diff accordindly.

199

Fair. Noted. Happy to update the commit message.

273

https://www.shellcheck.net/wiki/SC2166 which refers to upstream The Open Group Base Specifications https://pubs.opengroup.org/onlinepubs/9799919799/ which states (in its present version, Issue 8):

The -a and -o binary primaries and the '(' and ')' operators have been removed.

The previous issue (Issue 7) to which the shellcheck site links has the wording:

The XSI extensions specifying the -a and -o binary primaries and the '(' and ')' operators have been marked obsolescent.

So either way, I would argue the direction of travel has only hardened in favour of refactoring.

thebugfixers_pm.me edited the summary of this revision. (Show Details)

Restored localand added respective shellcheck disable=SC3043.
Updated commit summary re: shellcheck disable=SC3028