Aw: Re: [PATCH 2/3] network-functions.pl : Compare correct variables in network_equal

[Bernhard Bitsch <Bernhard.Bitsch@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 2834 bytes --]



> Gesendet: Montag, 06. Januar 2020 um 18:16 Uhr
> Von: "Michael Tremer" <michael.tremer(a)ipfire.org>
> An: "Tim FitzGeorge" <ipfr(a)tfitzgeorge.me.uk>
> Cc: development(a)lists.ipfire.org
> Betreff: Re: [PATCH 2/3] network-functions.pl : Compare correct variables in network_equal
>
> Okay.
> 
> Did I say that I hate Perl?
> 
> I suppose if you tested this, it works. And it must have worked at some point before, because there was a test suite.
> 

Not a testsuite, but formal verification with the language definition ( which is huge in case of Perl ;) ) proves the correctness in this special case.
SCNR

-Bernhard

> Acked-by: Michael Tremer <michael.tremer(a)ipfire.org>
> 
> -Michael
> 
> > On 28 Dec 2019, at 20:57, Tim FitzGeorge <ipfr(a)tfitzgeorge.me.uk> wrote:
> > 
> > Hello,
> > 
> > On 24/12/2019 13:04, Michael Tremer wrote:
> >> Hello,
> >> 
> >> I am still not sure what you are trying to solve here.
> >> 
> >> &network2bin() has two possible return values:
> >> 
> >> 1) An array with two values
> >> 
> >> Or
> >> 
> >> 2) Undefined
> > 
> > No. It's called in array context so the return value in the latter case
> > is the list ( undef ).
> > 
> >> 
> >> You are now checking if you have an array with any length but two, but this won’t work for undefined. That should at least print a warning that you are trying to determine the length of an undefined array.
> >> 
> >> So why is this change needed?
> > 
> > Partly because the return value is an array and partly because the
> > original code checked the scalar variables $bin1 and $bin2 rather than
> > the arrays @bin1 and @bin2 containing the return values; since these
> > scalars aren't used anywhere, network_equal() always returned undef,
> > which is actually the root cause of my original problem.
> > 
> > Tim
> > 
> >> 
> >> -Michael
> >> 
> >>> On 24 Dec 2019, at 13:56, Tim FitzGeorge <ipfr(a)tfitzgeorge.me.uk> wrote:
> >>> 
> >>> Check result of network2bin is correct rather than checking non-existent
> >>> variable.
> >>> 
> >>> Signed-off-by: Tim FitzGeorge <ipfr(a)tfitzgeorge.me.uk>
> >>> ---
> >>> config/cfgroot/network-functions.pl | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>> 
> >>> diff --git a/config/cfgroot/network-functions.pl b/config/cfgroot/network-functions.pl
> >>> index 56b4bceb7..a3f574760 100644
> >>> --- a/config/cfgroot/network-functions.pl
> >>> +++ b/config/cfgroot/network-functions.pl
> >>> @@ -111,7 +111,7 @@ sub network_equal {
> >>> 	my @bin1 = &network2bin($network1);
> >>> 	my @bin2 = &network2bin($network2);
> >>> 
> >>> -	if (!defined $bin1 || !defined $bin2) {
> >>> +	if (@bin1 != 2 || @bin2 != 2) {
> >>> 		return undef;
> >>> 	}
> >>> 
> >>> -- 
> >>> 2.16.4
> >>> 
> >> 
> > 
> 
>