aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVratko Polak <vrpolak@cisco.com>2019-02-19 15:01:34 +0100
committerVratko Polak <vrpolak@cisco.com>2019-02-27 10:26:43 +0100
commit6ac7b25a4c1aacfde41331bba651ab9bad9eb88e (patch)
tree63595df7b96ef76c3ec70973b816aec17d8712f0
parent694b418272e9d7670ac69d477ed731bb7445b65a (diff)
Bash style: Add rules for handling working dir
+ Also added details related to "set -e" and exit codes. Change-Id: I731151efd7facc1def3c8cbba4e466e793c93689 Signed-off-by: Vratko Polak <vrpolak@cisco.com>
-rw-r--r--docs/bash_code_style.rst94
1 files changed, 94 insertions, 0 deletions
diff --git a/docs/bash_code_style.rst b/docs/bash_code_style.rst
index 5a73d7b736..e5dbc9c06e 100644
--- a/docs/bash_code_style.rst
+++ b/docs/bash_code_style.rst
@@ -120,6 +120,10 @@ Safety
When was the last time you checked error code of "echo" command,
for example?
+ + Another example: "set -e" in your function has no effect
+ if any ancestor call is done with logical or,
+ for example in "func || code=$?" construct.
+
+ As there is no reliable method of error detection, and there are two
largely independent unreliable methods, the best what we can do
is to apply both. So, code SHOULD explicitly
@@ -162,9 +166,27 @@ Bash options
+ Code MUST to store "$?" and call "set -e" immediatelly afterwards.
+ + Code MUST NOT use this approach when calling functions.
+
+ + That is because functions are instructed to apply "set -e" on their own
+ which (when triggered) will exit the whole entry script.
+
+ + Unless overriden by ERR trap.
+ But code SHOULD NOT set any ERR trap.
+
+ + If code needs exit code of a function, it is RECOMMENDED to use
+ pattern 'code="0"; called_function || code="${?}"'.
+
+ + In this case, contributor MUST make sure nothing in the
+ called_function sub-graph relies on "set -e" behavior,
+ because the call being part of "or construct" disables it.
+
+ Code MAY append "|| true" for benign commands,
when it is clear non-zero exit codes make no difference.
+ + Also in this case, the contributor MUST make sure nothing within
+ the called sub-graph depends on "set -e", as it is disabled.
+
+ Code MUST apply "-u" as unset variable is generally a typo, thus an error.
+ Code MAY temporarily apply "+u" if a command needs that to pass.
@@ -377,6 +399,78 @@ Arguments or variables
+ For most Linux distros, "getopt" is RECOMMENDED.
+Working directory handling
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
++ Functions SHOULD act correctly without neither assuming
+ what the currect working directory is, nor changing it.
+
+ + That is why global variables and arguments SHOULD contain
+ (normalized) full paths.
+
+ + Motivation: Different call sites MAY rely on different working directories.
+
++ A function MAY return (also with nonzero exit code) when working directory
+ is changed.
+
+ + In this case the function documentation MUST clearly state where (and when)
+ is the working directory changed.
+
+ + Exception: Functions with undocumented exit code.
+
+ + Those functions MUST return nonzero code only on "set -e" or "die".
+
+ + Note that both "set -e" and "die" by default result in exit of the whole
+ entry script, but the caller MAY have altered that behavior
+ (by registering ERR trap, or redefining die function).
+
+ + Any callers which use "set +e" or "|| true" MUST make sure
+ their (and their caller ancestors') assumption on working directory
+ are not affected.
+
+ + Such callers SHOULD do that by restoring the original working directory
+ either in their code,
+
+ + or contributors SHOULD do such restoration in the function code,
+ (see below) if that is more convenient.
+
+ + Motivation: Callers MAY rely on this side effect to simplify their logic.
+
++ A function MAY assume a particular directory is already set
+ as the working directory (to save space).
+
+ + In this case function documentation MUST clearly state what the assumed
+ working directory is.
+
+ + Motivation: Callers MAY call several functions with common
+ directory of interest.
+
+ + Example: Several dowload actions to execute in sequence,
+ implemented as functions assuming ${DOWNLOAD_DIR}
+ is the working directory.
+
++ A function MAY change the working directory transiently,
+ before restoring it back before return.
+
+ + Such functions SHOULD use command "pushd" to change the working directory.
+
+ + Such functions SHOULD use "trap 'trap - RETURN; popd' RETURN"
+ imediately after the pushd.
+
+ + In that case, the "trap - RETURN" part MUST be included,
+ to restore any trap set by ancestor.
+
+ + Functions MAY call "trap - RETURN; popd" exlicitly.
+
+ + Such functions MUST NOT call another pushd (before an explicit popd),
+ as traps do not stack within a function.
+
++ If entry scripts also use traps to restore working directory (or other state),
+ they SHOULD use EXIT traps instead.
+
+ + That is because "exit" command, as well as the default behavior
+ of "die" or "set -e" cause direct exit (without skipping function returns).
+
Function size
~~~~~~~~~~~~~