On Mon, 5 Apr 2010, Damyan Ivanov wrote:
> -=| Cristian Ionescu-Idbohrn, Mon, Apr 05, 2010 at 10:56:20AM +0200 |=-
> >
> > Should I post patches to this list, or shall we take that off list?
>
> List is fine.
>
> > Do you like big or small patches?
>
> Small, self-contained.
>
> > All tested or even untested? I don't think I can test all, as the
> > hardware I own does not have all "features".
> > And/or bugs :)
>
> Test the ones you can. Be careful with the rest.
Alright.
The attached pach cleans up whitespace, adjusts indentation, removes some
unnecessary quoting, declares a local variable, rewords a comment and
removes another. Not too controversial, I would hope :)
Observations:
* 'functions.sh' uses '/bin/ps' in 'detect_x_display', but the package
doesn't depend on 'procps'
* 'functions.sh' uses '/bin/fgconsole' in 'detect_x_display', but the
package doesn't depend on 'console-tools' (yes, the package depends on
'acpi-support-base', which depends on 'console-tools', but is that a
reliable dependency?
* most functions do _not_ validate arguments before using them; that opens
for subtle bugs being more difficult to locate
* I'd like to suggest a function to be used in conjunction with error
handling, using '/usr/bin/logger' instead of echo; something along these
lines:
SELF="${0##*/}[$$]"
logit() {
[ $# -gt 1 ] || {
echo "$SELF: logit: missing argument(s)" >&2
exit 1
}
logger -t $SELF -p $1 -- $*
}
* it's difficult to distinguish between global and (undeclared) local
variables; what about upper case _only_ for global variables and lower
case for the local ones (see f.ex. function 'detect_x_display':
user=$_user
home=$_home
are 'user' and 'home' global or local variables?)
* functions that update global/the caller's variables open a can of worms,
IMO; that should be avoided; telling the function the variable name is a
better way (TM) to do it; something like:
func() {
# $1 - name of the variable to be updated
[ $# -ge 1 ] ||
echo "$SELF: func: missing argument(s)" >&2
exit 1
}
...
eval $1=...
}
* rfkill _does_ return error messages:
# rfkill list otto
Bogus list argument 'otto'.
$ echo $?
0
# rfkill list ""
Bogus list argument ''.
# echo $?
0
but not error status; I propose filing a bug against rfkill
* is there a good reason for running some of the programs using the whole
path (/usr/bin/gnome-screensaver-command, /usr/bin/xtrlock, etc.) but
not others (cut, who, getent, dcop, etc.)
Cheers,
--
Cristiandiff --git a/functions.sh b/functions.sh
index e17cf6b..661f3f6 100644
--- a/functions.sh
+++ b/functions.sh
@@ -1,9 +1,8 @@
# common eeepc-acpi-scripts functions
-# detect which rfkill has name=$1
have_dev_rfkill()
{
- [ -c /dev/rfkill ]
+ [ -c /dev/rfkill ]
}
if have_dev_rfkill; then
@@ -12,9 +11,9 @@ if have_dev_rfkill; then
{
# expecting something like
# 0: eeepc-wlan: Wireless LAN
- RFKILL=''
+ RFKILL=
if test "$(rfkill list "$1")" != ''; then
- RFKILL="$1"
+ RFKILL=$1
fi
}
@@ -32,8 +31,7 @@ if have_dev_rfkill; then
rfkill unblock "$1"
fi
}
-else # we have no /dev/rfkill
-
+else # no rfkill device
# detect which rfkill has name=$1
detect_rfkill()
{
@@ -41,11 +39,11 @@ else # we have no /dev/rfkill
for _rfkill in /sys/class/rfkill/*; do
if [ -f "$_rfkill/name" ] && [ "$(cat "$_rfkill/name")" = "$1" ]; then
echo "Detected $1 as rfkill $_rfkill" >&2
- RFKILL="$_rfkill/state"
+ RFKILL=$_rfkill/state
return
fi
done
- RFKILL=''
+ RFKILL=
}
get_rfkill()
@@ -63,62 +61,63 @@ detect_x_display()
{
local _user
local _home
- _user=$(who | sed -n '/ (:0[\.0]*)$\| :0 /{s/ .*//p;q}')
+ _user=$(who | sed -ne '/ (:0[\.0]*)$\| :0 /{s/ .*//p;q}')
if [ "$_user" = "" ]; then
- # no users seem to be logged on a X display?
- # try the first logged user without any filters
- # useful for users starting X via 'startx' after logging
- # on the console
- #_user=$( who | head -n 1 | cut -d' ' -f1 )
- _user=$(ps -o pid= -t tty$(fgconsole) | sed -e 's/^\s\+//g' | cut -d' ' -f1)
- if [ "${_user}" != '' ]; then
- eval $(sed -e 's/\x00/\n/g' /proc/${_user}/environ | grep '^\(DISPLAY\|XAUTHORITY\)=' | sed -e "s/'/'\\\\''/g; s/=/='/; s/$/'/")
- DISPLAY="${DISPLAY:-:0}"
- export XAUTHORITY
- export DISPLAY
- user=root
- home=$(getent passwd $_user | cut -d: -f6)
- fi
- return
+ # no users seem to be logged on the X display?
+ # try the first logged user without any filters
+ # useful for users starting X via 'startx' after logging
+ # on the console
+ #_user=$( who | head -n 1 | cut -d' ' -f1 )
+ _user=$(ps -o pid= -t tty$(fgconsole) | sed -e 's/^\s\+//g' | cut -d' ' -f1)
+ if [ "$_user" != '' ]; then
+ eval $(sed -e 's/\x00/\n/g' /proc/$_user/environ | grep '^\(DISPLAY\|XAUTHORITY\)=' | sed -e "s/'/'\\\\''/g; s/=/='/; s/$/'/")
+ DISPLAY=${DISPLAY:-:0}
+ export XAUTHORITY
+ export DISPLAY
+ user=root
+ home=$(getent passwd $_user | cut -d: -f6)
+ fi
+ return
fi
_home=$(getent passwd $_user | cut -d: -f6)
XAUTHORITY=$_home/.Xauthority
if [ -f "$XAUTHORITY" ]; then
- export XAUTHORITY
- export DISPLAY=:0
- user=$_user
- home=$_home
+ export XAUTHORITY
+ export DISPLAY=:0
+ user=$_user
+ home=$_home
fi
}
shell_quote()
{
- echo "$1" | sed -e 's/'\''/'\''\\'\'''\''/g; s/^/'\''/; s/$/'\''/; $! s/$/\\/'
+ echo "$1" | sed -e 's/'\''/'\''\\'\'''\''/g; s/^/'\''/; s/$/'\''/; $! s/$/\\/'
}
lock_x_screen()
{
# activate screensaver if available
+ local ss
if [ -S /tmp/.X11-unix/X0 ]; then
- detect_x_display
- if [ -f "$XAUTHORITY" ]; then
- for ss in xscreensaver gnome-screensaver; do
- [ -x /usr/bin/$ss-command ] \
- && pidof /usr/bin/$ss > /dev/null \
- && su - "$user" -c "DISPLAY=$(shell_quote "$DISPLAY") XAUTHORITY=$(shell_quote "$XAUTHORITY") $ss-command --lock"
- done
- # try locking KDE
- if [ -x /usr/bin/dcop ]; then
- dcop --user $user kdesktop KScreensaverIface lock
- fi
- [ -x /usr/bin/xtrlock ] && su - "$user" -c "DISPLAY=$(shell_quote "$DISPLAY") XAUTHORITY=$(shell_quote "$XAUTHORITY") /usr/bin/xtrlock" &
- fi
+ detect_x_display
+ if [ -f "$XAUTHORITY" ]; then
+ for ss in xscreensaver gnome-screensaver; do
+ [ -x /usr/bin/$ss-command ] \
+ && pidof /usr/bin/$ss > /dev/null \
+ && su - "$user" -c "DISPLAY=$(shell_quote "$DISPLAY") XAUTHORITY=$(shell_quote "$XAUTHORITY") $ss-command --lock"
+ done
+ # try locking KDE
+ if [ -x /usr/bin/dcop ]; then
+ dcop --user $user kdesktop KScreensaverIface lock
+ fi
+ [ -x /usr/bin/xtrlock ] && su - "$user" -c "DISPLAY=$(shell_quote "$DISPLAY") XAUTHORITY=$(shell_quote "$XAUTHORITY") /usr/bin/xtrlock" &
+ fi
fi
}
wakeup_wicd() {
WICD_WAKEUP=/usr/share/wicd/autoconnect.py
if [ -x $WICD_WAKEUP ]; then
- $WICD_WAKEUP
+ $WICD_WAKEUP
fi
}
_______________________________________________
Debian-eeepc-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/debian-eeepc-devel
