#102 Logwatch.pm ignores caller's my $DoLookup variable

[Daniel Lewart]

Logwatch Developers,

The Herculean changes for use strict are good!
However, Logwatch.pm ignores the caller's my $DoLookup variable,
which is lexically local.

The root cause is that Logwatch.pm uses global variables.
Good practice is to replace global variables with lexical variables and accessors.

Patch is attached.

Thank you!
Daniel Lewart
Urbana, Illinois

[Bjorn]

Thanks, Daniel, for the proposed patch. I did want to make some comments to the wider users of Logwatch.

• I agree that declaring lexical variables is a better approach for Perl.
• I see that 21 of the service scripts use the LookupIP function.
• But only six appear to set the DoLookup variable. In addition to the four you patched, those include 'courier' and 'pound'.
• Services 'named' and 'secure' have their DoLookup assignments commented out; don't know why.
• I see that the library file Logwatch.pm sets the default to do reverse DNS lookups, but most of the scripts (except for 'pound' and 'sshd') default to no lookups. I do remember some discussion where it was felt that no lookups was the preferred default, as it is time-consuming to do many lookups, and oftentimes the result is of little value, especially when they come from dynamic IPs, or from hosts with no reverse IPs.

I note these items, and wanted to highlight this proposed change to a wider audience. The current scheme is for the LookupIP function to reach into the %main:: namespace and check for the declaration of the DoLookup variable. (Don't know the history behind this. It's clever, but convoluted.)

Where it might break things is if someone created their own private scripts that use the existing DoLookup mechanism. It would break with the current patches. The fix would be similar to that of the scripts included in the patch file.

Also, if we apply these patches, we should clean up the other scripts to use new DoLookup function.

So I am looking for comments/opinions.

[Bjorn]

Patch applied. Thanks for the patch, Daniel.