[Debian-eeepc-devel] eeepc-acpi-scripts + console + gnu screen ircase?effect

[Cristian Ionescu-Idbohrn]

On Mon, 12 Apr 2010, Darren Salt wrote:

> I demand that Cristian Ionescu-Idbohrn may or may not have written...
>
> [snip]
> > Is writing to /dev/console a good idea, at all?  joeuser is not
> > allowed to do that.  This is how that file is listed on my box:
>
> > crw------- 1 root root 5, 1 Mar 15 09:23 /dev/console
>
> Probably not a good idea, but if writing to stdout won't work...

You can always use logger instead.

> > I also see this (on line 9):
> > 	echo "usage: notify 'category' 'message text' [fast]" > /dev/stderr
> > Shouldn't that be:
> > 	echo "usage: notify 'category' 'message text' [fast]" >&2
>
> Both will work equally well, given that /dev/stderr is a symlink
> pointing to the current process's fd 2.

Sure.  But why use the 'unconvensional' redirect to /dev/stderr when
there is one specially designed.  Information on this page:

	http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_07

may help.

> > This:
> > 	if [ -n "$4" -o \( -n "$3" -a "$3" != 'fast' \) ]; then
> > 	                 ^                            ^
> > will fork a subshell.
>
> No; but, unescaped, it would.

I'll have to dig a little.  What shell?  Bash or Dash?

> > What does $3 represent?  I see it's tested against the string 'fast'.
> > I guess $3 may also have the value 'low' :)
>
> Not really...

My typo, I intended to write 'slow' :)

> > Which variables are global?  Which are local?
>
> It seems safe to say that all variables declared in that function are
> intended to be local.

In that case it would be better to declare them as such.  That will
avoid cluttering the name space and the risk to overwrite other
variables in the callers environment too.

There's a global variable '$user' which is set in the
'detect_x_display' function.

> > How smart is to first initialize a variable to the null string:
>
> > 25	OSD_SHOWN=
> > 26
> > 27	# try to show a nice OSD notification via GNOME OSD service
> > 28	GOSDC=/usr/bin/gnome-osd-client
>
> > and then check if it's set to the null string?
>
> > 29	if [ -z "$OSD_SHOWN" ] && [ -x "$GOSDC" ]; then
>
> See commit a0892a9b; consider if code which may set that is inserted
> between the initial assignment and that test, and later (re)moved.

You mean: "test for OSD_SHOWN before checking for gnome-osd for
consistency reasons"?

Yes, well, there may be a point with that.  Won't insist.  Still,
there's only 4 lines between assignment and test.


Cheers,

-- 
Cristian