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: 2210 bytes --]

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.

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
>>> 
>> 
>