[Logcheck-devel] Bug#272047: logcheck's greplogoutput code needs cleaning

[Ross Boylan]

Package: logcheck
Version: 1.2.25
Severity: normal

Unless my understanding of shell scripts is off, there seem to be
quite a few problems with the greplogoutput routine.  First,
individual sections of the code contain comments that clearly do not
match what the code is doing.  Second, the overall sequencing and flow
of the tests (as written--perhaps less so as commented) doesn't make
sense.

Consider
		# Now ignore all entries from the logcheck-<package> files
		if [ -n "$ignore" ]; then
		    debug "Applying Logcheck override files"
		    for file in $(ls -1 $ignore/) ; do
		        debug "clean logcheck-<package>: $file"
			cleanchecked "$ignore/$file"
		    done
		else
The code is actually using every single ignore file, though the
comments say it should be using only the logcheck-* files.

Given that, the subsequent efforts to pick out various subsets of the
ignore files are completely redundant.

Now even if only logcheck-* were run, the subsequent section

		debug "Cleaning logcheck"
		# Remove any  entries already reported
		for file in $(ls $raise/ | grep -v '^logcheck') ; do
		    debug "Cleaning logcheck: $file"
		    cleanchecked "$raise/$file"
		done
doesn't make much sense.  The very first action of greplogoutput is to
look for an ignore file of the same name, which in this case must be
logcheck.  The previous code, if it followed the comments, would have
got all the logcheck-* files.  So this section redoes all those things
that have already been done.  It's only "contribution" is to pick up
logcheck* files that aren't logcheck-*, and it's not clear to me
that's desirable (or that there are likely to be any such files).

Finally, the overall logic according to the comments seems
unnecessarily complex.  For every package foo, the ignore files named
foo
logcheck-foo
local
local-*
are ignored.  Why not local-foo instead of local-*?

Then there is the added wrinkle of the separate handling when the
"raise" file name is logcheck.

I encourage you to decide what you want this code to do  and then make
the code and the comments match that behavior.  Finally, *please*
expand the manual page to document what the behavior is.  My patch in
bug #215640 provides such a revised man page.  I believe it reflected
the actual behavior of the code at one point; there may have been
changes since, and it will need to reflect whatever decisions are made
dealing with the issues raised above.

I really would like to avoid the necessity of rereading the code every
time I want to figure out how to fill in the configuration files :)

-- System Information:
Debian Release: 3.1
  APT prefers testing
  APT policy: (990, 'testing'), (50, 'unstable')
Architecture: i386 (i686)
Kernel: Linux 2.4.26advncdfs
Locale: LANG=en_US, LC_CTYPE=en_US

Versions of packages logcheck depends on:
ii  adduser          3.59                    Add and remove users and groups
ii  cron             3.0pl1-86               management of regular background p
ii  debconf [debconf 1.4.30.3                Debian configuration management sy
ii  debianutils      2.8.4                   Miscellaneous utilities specific t
ii  exim [mail-trans 3.36-11                 An MTA (Mail Transport Agent)
ii  lockfile-progs   0.1.10                  Programs for locking and unlocking
ii  logcheck-databas 1.2.25                  A database of system log rules for
ii  logtail          1.2.25                  Print log file lines that have not
ii  mailx            1:8.1.2-0.20040524cvs-1 A simple mail user agent
ii  perl             5.8.4-2                 Larry Wall's Practical Extraction 
ii  sysklogd [system 1.4.1-15                System Logging Daemon

-- debconf information:
* logcheck/security_level: workstation
* logcheck/manage_conffiles: true
* logcheck/auto_create_logfiles: true
  logcheck/upgrade-note:
* logcheck/changes:
* logcheck/install-note:
* logcheck/rules-directories-note:
* logcheck/email_address: root
* logcheck/rewrite-note: