| Message ID | 20191224125652.12232-3-ipfr@tfitzgeorge.me.uk |
|---|---|
| State | Dropped |
| Headers |
Return-Path: <development-bounces@lists.ipfire.org> Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) client-signature ECDSA (P-384)) (Client CN "mail01.haj.ipfire.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by web04.haj.ipfire.org (Postfix) with ESMTPS id 47hx7q2hHRz3xXr for <patchwork@web04.haj.ipfire.org>; Tue, 24 Dec 2019 12:57:11 +0000 (UTC) Received: from mail02.haj.ipfire.org (mail02.haj.ipfire.org [172.28.1.201]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384 client-signature ECDSA (P-384) client-digest SHA384) (Client CN "mail02.haj.ipfire.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mail01.ipfire.org (Postfix) with ESMTPS id 47hx7q0Sg5z2JW; Tue, 24 Dec 2019 12:57:11 +0000 (UTC) Received: from mail02.haj.ipfire.org (localhost [127.0.0.1]) by mail02.haj.ipfire.org (Postfix) with ESMTP id 47hx7p6JgPz2yCJ; Tue, 24 Dec 2019 12:57:10 +0000 (UTC) Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) client-signature ECDSA (P-384)) (Client CN "mail01.haj.ipfire.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mail02.haj.ipfire.org (Postfix) with ESMTPS id 47hx7n1vThz2xk6 for <development@lists.ipfire.org>; Tue, 24 Dec 2019 12:57:09 +0000 (UTC) Received: from mail-out-auth3.hosts.co.uk (mail-out-auth3.hosts.co.uk [85.233.191.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (Client did not present a certificate) by mail01.ipfire.org (Postfix) with ESMTPS id 47hx7m6vTKz2JW for <development@lists.ipfire.org>; Tue, 24 Dec 2019 12:57:08 +0000 (UTC) Received: from [95.149.142.227] (helo=aragorn.hosts.co.uk.tfitzgeorge.me.uk) by smtp.hosts.co.uk with esmtpa (Exim) (envelope-from <ipfr@tfitzgeorge.me.uk>) id 1ijjkD-0006qq-Ar; Tue, 24 Dec 2019 12:57:01 +0000 From: Tim FitzGeorge <ipfr@tfitzgeorge.me.uk> To: development@lists.ipfire.org Subject: [PATCH 2/3] network-functions.pl : Compare correct variables in network_equal Date: Tue, 24 Dec 2019 12:56:51 +0000 Message-Id: <20191224125652.12232-3-ipfr@tfitzgeorge.me.uk> X-Mailer: git-send-email 2.16.4 In-Reply-To: <20191224125652.12232-1-ipfr@tfitzgeorge.me.uk> References: <20191224125652.12232-1-ipfr@tfitzgeorge.me.uk> Authentication-Results: mail01.ipfire.org; dkim=none; dmarc=none; spf=pass (mail01.ipfire.org: domain of ipfr@tfitzgeorge.me.uk designates 85.233.191.1 as permitted sender) smtp.mailfrom=ipfr@tfitzgeorge.me.uk X-Rspamd-Queue-Id: 47hx7m6vTKz2JW X-Spamd-Result: default: False [-3.22 / 11.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; RCVD_IN_DNSWL_LOW(-0.10)[85.233.191.1:from]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; R_SPF_ALLOW(-0.20)[+ip4:85.233.191.0/25]; MIME_GOOD(-0.10)[text/plain]; SENDER_REP_HAM(0.00)[asn: 8622(-0.30), country: GB(-0.00), ip: 85.233.191.1(0.00)]; DMARC_NA(0.00)[tfitzgeorge.me.uk]; RECEIVED_SPAMHAUS_PBL(0.00)[95.149.142.227:received]; TO_MATCH_ENVRCPT_SOME(0.00)[]; MX_GOOD(-0.01)[cached: mx1.ukservers.net]; RCPT_COUNT_TWO(0.00)[2]; MID_CONTAINS_FROM(1.00)[]; NEURAL_HAM(-0.82)[-0.818]; FROM_EQ_ENVFROM(0.00)[]; RCVD_TLS_LAST(0.00)[]; R_DKIM_NA(0.00)[]; ASN(0.00)[asn:8622, ipnet:85.233.160.0/19, country:GB]; MIME_TRACE(0.00)[0:+]; BAYES_HAM(-3.00)[99.98%]; RCVD_COUNT_TWO(0.00)[2] X-Rspamd-Server: mail01.haj.ipfire.org X-BeenThere: development@lists.ipfire.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: IPFire development talk <development.lists.ipfire.org> List-Unsubscribe: <https://lists.ipfire.org/mailman/options/development>, <mailto:development-request@lists.ipfire.org?subject=unsubscribe> List-Archive: <http://lists.ipfire.org/pipermail/development/> List-Post: <mailto:development@lists.ipfire.org> List-Help: <mailto:development-request@lists.ipfire.org?subject=help> List-Subscribe: <https://lists.ipfire.org/mailman/listinfo/development>, <mailto:development-request@lists.ipfire.org?subject=subscribe> Errors-To: development-bounces@lists.ipfire.org Sender: "Development" <development-bounces@lists.ipfire.org> |
| Series |
network-functions.pl : Correct comparisons and enhance testsuite
|
|
Commit Message
Tim FitzGeorge
Dec. 24, 2019, 12:56 p.m. UTC
Check result of network2bin is correct rather than checking non-existent
variable.
Signed-off-by: Tim FitzGeorge <ipfr@tfitzgeorge.me.uk>
---
config/cfgroot/network-functions.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
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 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? -Michael > On 24 Dec 2019, at 13:56, Tim FitzGeorge <ipfr@tfitzgeorge.me.uk> wrote: > > Check result of network2bin is correct rather than checking non-existent > variable. > > Signed-off-by: Tim FitzGeorge <ipfr@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 >
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@tfitzgeorge.me.uk> wrote: >> >> Check result of network2bin is correct rather than checking non-existent >> variable. >> >> Signed-off-by: Tim FitzGeorge <ipfr@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 >> >
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@ipfire.org> -Michael > On 28 Dec 2019, at 20:57, Tim FitzGeorge <ipfr@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@tfitzgeorge.me.uk> wrote: >>> >>> Check result of network2bin is correct rather than checking non-existent >>> variable. >>> >>> Signed-off-by: Tim FitzGeorge <ipfr@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 >>> >> >
> Gesendet: Montag, 06. Januar 2020 um 18:16 Uhr > Von: "Michael Tremer" <michael.tremer@ipfire.org> > An: "Tim FitzGeorge" <ipfr@tfitzgeorge.me.uk> > Cc: development@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@ipfire.org> > > -Michael > > > On 28 Dec 2019, at 20:57, Tim FitzGeorge <ipfr@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@tfitzgeorge.me.uk> wrote: > >>> > >>> Check result of network2bin is correct rather than checking non-existent > >>> variable. > >>> > >>> Signed-off-by: Tim FitzGeorge <ipfr@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 > >>> > >> > > > >
Thx. > On 6 Jan 2020, at 17:33, Bernhard Bitsch <Bernhard.Bitsch@gmx.de> wrote: > > > >> Gesendet: Montag, 06. Januar 2020 um 18:16 Uhr >> Von: "Michael Tremer" <michael.tremer@ipfire.org> >> An: "Tim FitzGeorge" <ipfr@tfitzgeorge.me.uk> >> Cc: development@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@ipfire.org> >> >> -Michael >> >>> On 28 Dec 2019, at 20:57, Tim FitzGeorge <ipfr@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@tfitzgeorge.me.uk> wrote: >>>>> >>>>> Check result of network2bin is correct rather than checking non-existent >>>>> variable. >>>>> >>>>> Signed-off-by: Tim FitzGeorge <ipfr@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 >>>>> >>>> >>> >> >>
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; }