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

[Michael Tremer <michael.tremer@ipfire.org>]

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

Thx.

> On 6 Jan 2020, at 17:33, Bernhard Bitsch <Bernhard.Bitsch(a)gmx.de> wrote:
> 
> 
> 
>> 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
>>>>> 
>>>> 
>>> 
>> 
>>