Tom Gundersen, 2011-06-25 21:10:
On Jun 25, 2011, at 20:11, Dave Reisner<[email protected]> wrote:
On Sat, Jun 25, 2011 at 12:15:56PM +0200, Kurt J. Bosch wrote:
---
functions | 50 +++++++++++++++++++++++++-------------------------
1 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/functions b/functions
index 61cf3dd..9c881c1 100644
--- a/functions
+++ b/functions
@@ -285,17 +285,17 @@ kill_everything() {
# Terminate all processes
stat_busy "Sending SIGTERM To Processes"
run_hook "$1_prekillall"
- local pid k5args=""
- for pid in ${omit_pids[@]}; do
- k5args+=" -o $pid"
- done
- /sbin/killall5 -15 $k5args&>/dev/null
- /bin/sleep 5
+ local pid k5args=""
+ for pid in ${omit_pids[@]}; do
+ k5args+=" -o $pid"
+ done
I'd rather just see this whole chunk of code go away. Building the extra
variable is moot when you can just use a PE:
killall 5 -15 ${omit_pids[@]/#/-o }
d
+ /sbin/killall5 -15 $k5args&>/dev/null
+ /bin/sleep 5
stat_done
stat_busy "Sending SIGKILL To Processes"
- /sbin/killall5 -9 $k5args&>/dev/null
- /bin/sleep 1
+ /sbin/killall5 -9 $k5args&>/dev/null
+ /bin/sleep 1
stat_done
run_hook "$1_postkillall"
@@ -371,13 +371,13 @@
NETFS="nonfs,nonfs4,nosmbfs,nocifs,nocodafs,noncpfs,nosysfs,noshfs,nofuse,nofuse
# Check local filesystems
fsck_all() {
stat_busy "Checking Filesystems"
- FSCK_OUT=/dev/stdout
- FSCK_ERR=/dev/stdout
- FSCK_FD=
- FORCEFSCK=
- [[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline)&& FORCEFSCK="--
-f"
- run_hook sysinit_prefsck
- fsck -A -T -C$FSCK_FD -a -t "$NETFS,noopts=_netdev" $FORCEFSCK>$FSCK_OUT
2>$FSCK_ERR
+ FSCK_OUT=/dev/stdout
+ FSCK_ERR=/dev/stdout
+ FSCK_FD=
+ FORCEFSCK=
+ [[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline)&&
FORCEFSCK="-- -f"
+ run_hook sysinit_prefsck
+ fsck -A -T -C$FSCK_FD -a -t "$NETFS,noopts=_netdev" $FORCEFSCK>$FSCK_OUT
2>$FSCK_ERR
local fsckret=$?
if (( fsckret> 1 )); then
stat_fail
@@ -426,9 +426,9 @@ fsck_reboot() {
mount_all() {
stat_busy "Mounting Local Filesystems"
- run_hook sysinit_premount
- mount -a -t $NETFS -O no_netdev
- run_hook sysinit_postmount
+ run_hook sysinit_premount
+ mount -a -t $NETFS -O no_netdev
+ run_hook sysinit_postmount
stat_done
}
@@ -502,13 +502,13 @@ fi
set_consolefont() {
[[ $CONSOLEFONT ]] || return 0
stat_busy "Loading Console Font: $CONSOLEFONT"
- #CONSOLEMAP in UTF-8 shouldn't be used
- [[ $CONSOLEMAP&& ${LOCALE,,} =~ utf ]]&& CONSOLEMAP=""
- local i
- for i in /dev/tty[0-9]*; do
- /usr/bin/setfont ${CONSOLEMAP:+-m ${CONSOLEMAP}} \
- $CONSOLEFONT -C ${i}&>/dev/null
- done
+ #CONSOLEMAP in UTF-8 shouldn't be used
+ [[ $CONSOLEMAP&& ${LOCALE,,} =~ utf ]]&& CONSOLEMAP=""
+ local i
+ for i in /dev/tty[0-9]*; do
+ /usr/bin/setfont ${CONSOLEMAP:+-m ${CONSOLEMAP}} \
+ $CONSOLEFONT -C ${i}&>/dev/null
+ done
if (( $? )); then
stat_fail
elif [[ $CONSOLEMAP ]]; then
--
1.7.1
I never really understood why we indented like this. Since we seem to
be split 50/50, I'd argue that it might be better to just separate the
code with whitespace, e.g.
# more code above here...
stat_busy
foo
bar
baz
stat_done
# more code below here...
This way, we don't have to worry about what happens when there could be
a stat_fail somewhere in between that would make us have to wonder wtf
to do with indenting.
d
The reason for the indentation was to make it obvious how stat_{busy,fail,done}
match up. There were some errors due to mismatches before.
I think it is relatively clear how to DTRT: if stat_fail/stat_done is one
indentation level below stat_busy, then unindent it. Otherwise (e.g., if it is
inside a conditional), don't.
That said, I'm wondering if it wouldn't be clearer if we just used status, and
when needed put these code blocks inside functions.
-t
Hmm... Currently we have the checks for skipping the entire thing as
well as the stat_busy messages within the functions. This makes sense
for three reasons:
a) It allows to disable say set_consolefont in a sysinit hook by just
unsetting $CONSOLEFONT and then call it later via some rc.multi hook
without the need to duplicate the checking code (which isn't always that
simple). This actually happens to avoid destroying a splash screen right
in the middle of boot up.
b) It allows to override the check and also the message together with
the code instead of the code block only.
c) It allows to have more than one code block in one single function
(like in kill_everything)
So, when using status, we would end up with somthing like
set_consolefont() {
[[ $CONSOLEFONT ]] || return 0
status "Loading Console Font: $CONSOLEFONT" \
set_consolefont_inner
}
set_consolefont_inner() {
...
}
--
Kurt
