Bug#799097: mrtg-rrd: Regression after the fix for bug #787608.
On Friday, 13 January 2017 15:24:54 CET Damyan Ivanov wrote:
> Perhaps somebody from the perl group (CC-ed) can take a look?
See below...
> > > --- a/mrtg-rrd.cgi
> > > +++ b/mrtg-rrd.cgi
> > > @@ -496,7 +496,7 @@ sub common_args($$$)
> > >
> > > {
> > >
> > > my ($name, $target, $q) = @_;
> > >
> > > - return @{$target->{args}} if defined @{$target->{args}};
> > > + return @{$target->{args}} if exists $target->{args};
A more defensive way is
return @{$target->{args}} if ref($target->{args}) eq 'ARRAY';
> > >
> > > my $noi = 1 if $target->{options}{noi};
> > > my $noo = 1 if $target->{options}{noo};
> > >
> > > @@ -521,7 +521,7 @@ sub common_args($$$)
> > >
> > > $target->{rrd} = $dir . '/' . $tdir . $name . '.rrd';
> > >
> > > %{$target->{options}} = ()
> > >
> > > - unless defined %{$target->{options}};
> > > + unless %{$target->{options}};
unless ref(target->{options}) eq 'HASH'
> > >
> > > $dir = $cfg->{workdir};
> > > $dir = $cfg->{imagedir}
> > >
> > > @@ -908,7 +908,7 @@ EOF
> > >
> > > print $directories{$dir}{bodytag};
> > >
> > > my $subdirs_printed;
> > >
> > > - if (defined @{$directories{$dir}{subdir}}) {
> > > + if (exists $directories{$dir}{subdir}) {
if (ref($directories{$dir}{subdir}) eq 'ARRAY')
> > >
> > > $subdirs_printed = 1;
> > > print < > >
> > > MRTG subdirectories in the directory $dir1
> > >
> > > @@ -921,7 +921,7 @@ EOF
> > >
> > > print "\n";
> > >
> > > }
> > >
> > > - if (defined @{$directories{$dir}{target}}) {
> > > + if (exists $directories{$dir}{target}) {
if (ref($directories{$dir}{target}) eq 'ARRAY')
> > >
> > > print "\n" if defined $subdirs_printed;
> > > print < > >
> > > MRTG graphs in the directory $dir1
HTH
--
https://github.com/dod38fr/ -o- http://search.cpan.org/~ddumont/
http://ddumont.wordpress.com/ -o- irc: dod at irc.debian.org
Bug#799097: mrtg-rrd: Regression after the fix for bug #787608.
On Fri, 13 Jan 2017 15:24:54 +, Damyan Ivanov wrote:
> Probably, but I can't do it promptly. I don't agree with the statement
> that
>
>defined %{$hash{key}}
>
> is better to be replaced with
>
>exists $hash{key}
>
> than with
>
>%{$hash{key}}
>
> (same for defined @{$hash{key}})
Agreed.
I just found
http://haroon.sis.utoronto.ca/rrd/scripts/
https://github.com/corporate-gadfly/rrd.cgi
which seems to be a fork of mrtg-rrd, and the git repo contains the
following 2 commits:
https://github.com/corporate-gadfly/rrd.cgi/commit/0a32136461c3033a000ec497c0922e24b2b3acb9
https://github.com/corporate-gadfly/rrd.cgi/commit/489d0990211793cd438bc90f8616cb6877baee10
perldiag says:
Can't use 'defined(@array)' (Maybe you should just omit the defined()?)
(F) defined() is not useful on arrays because it checks for an
undefined scalar value. If you want to see if the array is empty, just
use "if (@array) { # not empty }" for example.
Can't use 'defined(%hash)' (Maybe you should just omit the defined()?)
(F) "defined()" is not usually right on hashes.
Although "defined %hash" is false on a plain not-yet-used hash, it
becomes true in several non-obvious circumstances, including
iterators, weak references, stash names, even remaining true after
"undef %hash". These things make "defined %hash" fairly useless in
practice, so it now generates a fatal error.
If a check for non-empty is what you wanted then just put it in
boolean context (see "Scalar values" in perldata):
if (%hash) {
# not empty
}
BTW 1: It would be nice to have actual error messages instead of "is
a blank page".
BTW 2, the last release of mrtg-rrd is from 2003:
https://www.fi.muni.cz/~kas/mrtg-rrd/
Cheers,
gregor
--
.''`. https://info.comodo.priv.at/ - Debian Developer https://www.debian.org
: :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06
`. `' Member of VIBE!AT & SPI, fellow of the Free Software Foundation Europe
`- NP: Various Artists: Couldn't Have Come At A Better Time
signature.asc
Description: Digital Signature
Bug#799097: mrtg-rrd: Regression after the fix for bug #787608.
-=| Adrian Bunk, 13.01.2017 17:01:15 +0200 |=-
> Damyan, can you take care of this?
Probably, but I can't do it promptly. I don't agree with the statement
that
defined %{$hash{key}}
is better to be replaced with
exists $hash{key}
than with
%{$hash{key}}
(same for defined @{$hash{key}})
I'd need to investigate what breaks, which would need time.
Perhaps somebody from the perl group (CC-ed) can take a look?
-- dam
> On Tue, Sep 15, 2015 at 11:08:53PM +0300, Vladimir Panov wrote:
> > Package: mrtg-rrd
> > Version: 0.7-5.1
> > Severity: grave
> > Tags: patch
> > Justification: renders package unusable
> >
> > Dear Maintainer,
> >
> > The fix for bug #787608 has left the package in an unusable state (the
> > result of the execution of mrtg-rrd.cgi is a blank page).
> > The cause is the blind removal of the defined function. At least in 3 of
> > the 4 instances it should be replaced with exists. I don't know about the
> > fourth instance since I am not a Perl expert.
> >
> > Please, see the proposed patch below. It should be applied instead of
> > no-defined-hash-array.patch, i.e. against the original source.
> >
> >
> > --- a/mrtg-rrd.cgi
> > +++ b/mrtg-rrd.cgi
> > @@ -496,7 +496,7 @@ sub common_args($$$)
> > {
> > my ($name, $target, $q) = @_;
> >
> > - return @{$target->{args}} if defined @{$target->{args}};
> > + return @{$target->{args}} if exists $target->{args};
> >
> > my $noi = 1 if $target->{options}{noi};
> > my $noo = 1 if $target->{options}{noo};
> > @@ -521,7 +521,7 @@ sub common_args($$$)
> > $target->{rrd} = $dir . '/' . $tdir . $name . '.rrd';
> >
> > %{$target->{options}} = ()
> > - unless defined %{$target->{options}};
> > + unless %{$target->{options}};
> >
> > $dir = $cfg->{workdir};
> > $dir = $cfg->{imagedir}
> > @@ -908,7 +908,7 @@ EOF
> > print $directories{$dir}{bodytag};
> >
> > my $subdirs_printed;
> > - if (defined @{$directories{$dir}{subdir}}) {
> > + if (exists $directories{$dir}{subdir}) {
> > $subdirs_printed = 1;
> > print < > MRTG subdirectories in the directory $dir1
> > @@ -921,7 +921,7 @@ EOF
> >
> > print "\n";
> > }
> > - if (defined @{$directories{$dir}{target}}) {
> > + if (exists $directories{$dir}{target}) {
> > print "\n" if defined $subdirs_printed;
> > print < > MRTG graphs in the directory $dir1
>
>
signature.asc
Description: Digital signature
Bug#799097: mrtg-rrd: Regression after the fix for bug #787608.
Damyan, can you take care of this?
Thanks
Adrian
On Tue, Sep 15, 2015 at 11:08:53PM +0300, Vladimir Panov wrote:
> Package: mrtg-rrd
> Version: 0.7-5.1
> Severity: grave
> Tags: patch
> Justification: renders package unusable
>
> Dear Maintainer,
>
> The fix for bug #787608 has left the package in an unusable state (the result
> of the execution of mrtg-rrd.cgi is a blank page).
> The cause is the blind removal of the defined function. At least in 3 of the
> 4 instances it should be replaced with exists. I don't know about the fourth
> instance since I am not a Perl expert.
>
> Please, see the proposed patch below. It should be applied instead of
> no-defined-hash-array.patch, i.e. against the original source.
>
>
> --- a/mrtg-rrd.cgi
> +++ b/mrtg-rrd.cgi
> @@ -496,7 +496,7 @@ sub common_args($$$)
> {
> my ($name, $target, $q) = @_;
>
> - return @{$target->{args}} if defined @{$target->{args}};
> + return @{$target->{args}} if exists $target->{args};
>
> my $noi = 1 if $target->{options}{noi};
> my $noo = 1 if $target->{options}{noo};
> @@ -521,7 +521,7 @@ sub common_args($$$)
> $target->{rrd} = $dir . '/' . $tdir . $name . '.rrd';
>
> %{$target->{options}} = ()
> - unless defined %{$target->{options}};
> + unless %{$target->{options}};
>
> $dir = $cfg->{workdir};
> $dir = $cfg->{imagedir}
> @@ -908,7 +908,7 @@ EOF
> print $directories{$dir}{bodytag};
>
> my $subdirs_printed;
> - if (defined @{$directories{$dir}{subdir}}) {
> + if (exists $directories{$dir}{subdir}) {
> $subdirs_printed = 1;
> print < MRTG subdirectories in the directory $dir1
> @@ -921,7 +921,7 @@ EOF
>
> print "\n";
> }
> - if (defined @{$directories{$dir}{target}}) {
> + if (exists $directories{$dir}{target}) {
> print "\n" if defined $subdirs_printed;
> print < MRTG graphs in the directory $dir1
Bug#799097: mrtg-rrd: Regression after the fix for bug #787608.
Package: mrtg-rrd
Version: 0.7-5.1
Severity: grave
Tags: patch
Justification: renders package unusable
Dear Maintainer,
The fix for bug #787608 has left the package in an unusable state (the result
of the execution of mrtg-rrd.cgi is a blank page).
The cause is the blind removal of the defined function. At least in 3 of the 4
instances it should be replaced with exists. I don't know about the fourth
instance since I am not a Perl expert.
Please, see the proposed patch below. It should be applied instead of
no-defined-hash-array.patch, i.e. against the original source.
--- a/mrtg-rrd.cgi
+++ b/mrtg-rrd.cgi
@@ -496,7 +496,7 @@ sub common_args($$$)
{
my ($name, $target, $q) = @_;
- return @{$target->{args}} if defined @{$target->{args}};
+ return @{$target->{args}} if exists $target->{args};
my $noi = 1 if $target->{options}{noi};
my $noo = 1 if $target->{options}{noo};
@@ -521,7 +521,7 @@ sub common_args($$$)
$target->{rrd} = $dir . '/' . $tdir . $name . '.rrd';
%{$target->{options}} = ()
- unless defined %{$target->{options}};
+ unless %{$target->{options}};
$dir = $cfg->{workdir};
$dir = $cfg->{imagedir}
@@ -908,7 +908,7 @@ EOF
print $directories{$dir}{bodytag};
my $subdirs_printed;
- if (defined @{$directories{$dir}{subdir}}) {
+ if (exists $directories{$dir}{subdir}) {
$subdirs_printed = 1;
print
