From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.haj.ipfire.org (localhost [127.0.0.1]) by mail02.haj.ipfire.org (Postfix) with ESMTP id 4ZBCd83Ztcz376V for ; Mon, 10 Mar 2025 10:22:36 +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 (secp384r1) server-digest SHA384 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mail01.haj.ipfire.org", Issuer "R10" (verified OK)) by mail02.haj.ipfire.org (Postfix) with ESMTPS id 4ZBCd45qplz2xc2 for ; Mon, 10 Mar 2025 10:22:32 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id 4ZBCd408Xjz7cX; Mon, 10 Mar 2025 10:22:31 +0000 (UTC) DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003ed25519; t=1741602152; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=606ollFtGnVdi45tfdgIvISoADFv7HaP04PGkuoAb6w=; b=HhiQE3trNNDJ6LkkP9ACn2bUhk9c6EMlufHZKpR1HMKvHU6Ux5Zj4Pae3vjudf0vZEEKxl x90z0i9q5YhBJnAA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003rsa; t=1741602152; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=606ollFtGnVdi45tfdgIvISoADFv7HaP04PGkuoAb6w=; b=jMc2+HuP9DO1fPD0tE117+1XsX0n4AtbETJMwKiapCd6k6e5fT+Ys9pT/dsFL6zLoH8S8D 7jLWB3zHIgmL5QfqIUBwZ4fDI9et0bLcMfpfNNUvsVCfL18xhP7/zfmBL2fpyuUeetcmwN xxuxw1ojU/uCixHlMcP0P0CWHt3yN7ENkTNonGirLa0nLqNL8AuyTPkWSnsDE/TVNAbGoQ sAgyIIwN6F8VFZR6WX2ug3SKYd+AOt2GCcMVoPKZShZGn40gfjwrPTHCIQ3ctdQ/5BxKKB 2X1rE1vygsCaMTY8hpnKnZ+w5ETcfkWE0jNM1Qa36JK8wZaYBPZdRQdaVWYDWw== Content-Type: text/plain; charset=utf-8 Precedence: list List-Id: List-Subscribe: , List-Unsubscribe: , List-Post: List-Help: Sender: Mail-Followup-To: Mime-Version: 1.0 Subject: Re: duplicate get_aliases subroutines found From: Michael Tremer In-Reply-To: <6e0d1e9a-3b62-4690-a6ba-a04628aaf9e2@ipfire.org> Date: Mon, 10 Mar 2025 10:22:31 +0000 Cc: "IPFire: Development-List" Content-Transfer-Encoding: quoted-printable Message-Id: <3A9860E4-6DDE-4722-9336-F4D6C364735B@ipfire.org> References: <6e0d1e9a-3b62-4690-a6ba-a04628aaf9e2@ipfire.org> To: Adolf Belka Hello Adolf, Welcome to the hell of the network code of IPFire 2. I am sure there are lot more places where the aliases table is parsed = but it is not even done in a function. This is why changing anything is = super painful because there so many places in the code that do the same = thing, but they don=E2=80=99t use a unique function. Therefore we will = have to find all these places and update them all the same time. And it = isn=E2=80=99t just copy & paste. There was the intention to clean this up properly, but we simply never = found the time for it. On the other hand, this works, and we don=E2=80=99t= intend to make too many changes in the future, so putting all of this = work in might never pay off. That statement comes with the limitation that there are of course hotter = paths in the code than others. Regarding this specific function, I first of all think it should live in = network-functions.pl, because it clearly is a networking function and is = not only being used in the IDS. The function in ids-functions.pl looks cleaner to me: = https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dblob;f=3Dconfig/cfgroot/ids= -functions.pl;hb=3D2112342dd3ccaf6008c742dddd4ca26b17c5651d#l1764 It even checks if the line can be read correctly and contains a valid IP = address. After parsing the file it returns an array. The function in general-functions.pl is a lot older: = https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dblob;f=3Dconfig/cfgroot/gen= eral-functions.pl;hb=3D2112342dd3ccaf6008c742dddd4ca26b17c5651d#l261 It parses the line, even adds some sort of prefix, but does not return = an array. Instead it populates a hash that has been declared somewhere = else. It returns nothing. It gets called in a few places and the data is = than being read from that globally declared array: = https://git.ipfire.org/?p=3Dipfire-2.x.git&a=3Dsearch&h=3D2112342dd3ccaf60= 08c742dddd4ca26b17c5651d&st=3Dgrep&s=3Dget_aliases This all would have to be changed if we wanted to adopt the better = version of this function in ids-functions.pl. Here is a list of other places where /var/ipfire/ethernet/aliases is = being parsed (never mind the shell parts): = https://git.ipfire.org/?p=3Dipfire-2.x.git&a=3Dsearch&h=3D2112342dd3ccaf60= 08c742dddd4ca26b17c5651d&st=3Dgrep&s=3Dethernet%2Faliases I know that the firewall code heavily relies on the prefix that is being = added in the one function, because the actual IP address (which would be = a unique key) is not being used when an alias is being selected in a = firewall rule. So long story short, I would be happy to see this cleaned up; but in = this rather simple case of a very straight-forward ten-ish lines long = function, you will probably have to spend the best part of a day to make = sure that no new regressions will be added. =E2=80=94 That being said, I think there are a lot more lower hanging fruits: = subtocidr/cidrtosub/iporsubtodec/iporsubtocidr/ip2dec/dec2ip/validipandmas= k and so on are only aliases that call the right function in = network-functions.pl. So those will just have to be found in the code = and replaced by the new function. Maybe that would be a good start to = get general-functions.pl smaller? Best, -Michael > On 8 Mar 2025, at 13:51, Adolf Belka wrote: >=20 > Hi All, >=20 > I was searching through the bugs list and found bug11393 about = refactoring the network functions. >=20 > Based on that I was having a look at what network functions were still = in general-functions.pl and on investigating I found that there is a sub = get_aliases in general-functions.pl and also in ids-functions.pl >=20 > They are clearly doing the same thing but with some differences. I am = not able to tell which is the better version to keep or if a version = combining aspects of both are needed. >=20 > If it is just a case of using one of them and putting it into the = network-functions.pl library and then updating all the references, then = I could do that without problems. If something needs to be changed then = I would need some guidance. >=20 > Of course the above also depends on get_aliases being considered to be = a network function, which was my interpretation. >=20 > I found quite a lot that are still in general-functions.pl >=20 > Regards, >=20 > Adolf. >=20 > --=20 > Sent from my laptop >=20 >=20