[Pm-utils] [PATCH] Logging infrastructure

[Stefan Seyfried]

On Sat, Oct 21, 2006 at 03:05:57PM -0400, Peter Jones wrote:
> On Fri, 2006-10-20 at 15:54 +0200, Stefan Seyfried wrote:
> 
> > Is this an acceptable idea? I explicitly went for the "as simple as possible"
> > approach, which also makes it trivial for hook writers to use it. All the
> > alternatives (a log() function or something like that) would have been more
> > complicated to use IMO.
> 
> In general, I think this is pretty good.
> 
> > I also removed all those "rm /var/run/pm-suspend" lines, since it does not
> > hurt to let this file lay around until the next suspend (of course it is
> > still deleted then before starting) and it also might contain various hints
> > on what went wrong.
> 
> I'd rather we just log what's in the file than leave it sitting
> around.  

Good, then i'll prepare some "verbosification" patches soon that log the stuff
that is put into pm-suspend.
If somebody really gets into trouble where the contents of /var/run/pm-suspend
are handy, he can trivially comment out the "rm" lines anyway.

> Some specific comments below.
> 
> ...
> >  pm_main()
> >  {
> > +	[ -n "$LOGFILE" ] && ![[ "$LOGFILE" =~ "^/dev/"  ] && rm -f $LOGFILE
> 
> Bad syntax...

Yes, i should probably run the code before submitting the patches... :-(

> > +	[ -n "$LOGFILE" ] && exec > $LOGFILE 2>&1
> 
> This is really ugly :/
> Why not:
> 
> if [ -n "$LOGFILE ]; then
>   [ -f "$LOGFILE" ] && rm -f $LOGFILE
>   touch "$LOGFILE"
> fi

It's not equivalent. "exec > $LOGFILE 2>&1" redirects all future stderr and
stdout to the logfile and that's what we want.
Also, the above (broken) line made sure that we do not do something stupid
if somebody puts in "/dev/null" as LOGFILE to make it stay quiet. Ok, test -f
does this also, so...

if [ -n "$LOGFILE ]; then
	[ -f "$LOGFILE ] && rm -f $LOGFILE
	exec > $LOGFILE 2>&1
fi

... should work as well.

If $LOGFILE is empty, then just do not redirect (basically for debugging by
running pm-hibernate manually from a root shell).
We could actually try to find out if we are running from HAL (=>redirect to
logfile) or not (interactive => show output on console), but that's not too
important yet.
 
> I'm not sure there's any point in redirecting the output of 'touch' to
> the logfile -- if touch fails, we can't write to the logfile anyway.

See above, it's not about the creation of the logfile, but all future echo's,
insmod errors or output from the "service $FOO restart" will end up in 
$LOGFILE.

-- 
Stefan Seyfried
QA / R&D Team Mobile Devices        |              "Any ideas, John?"
SUSE LINUX Products GmbH, Nürnberg  | "Well, surrounding them's out."