wido commented on code in PR #10594:
URL: https://github.com/apache/cloudstack/pull/10594#discussion_r2626101100


##########
scripts/vm/network/security_group.py:
##########
@@ -131,20 +134,37 @@ def ipv6_link_local_addr(mac=None):
     return ipaddress.ip_address('fe80::' + ':'.join(re.findall(r'.{4}', 
eui64)))
 
 
-def split_ips_by_family(ips):
-    if type(ips) is str:
-        ips = [ip for ip in ips.split(';') if ip != '']
+def split_ips_by_family(*addresses):
+    """
+    Takes one or more IP addresses as the input, and returns two lists:
+    one for ipv4 addresses, one for ipv6 addresses
+
+    If one of the inputs is a string, it tries to split it by semicolon,
+    and ignores the empty fields, or the fields with the value '0'
+    """
+    ips = []
+    for i in addresses:
+        if not i:  # IP address can be sometimes None (e.g. when ipv6 address 
not present)
+            continue
+        if type(i) is str:
+            ips += [ip for ip in i.split(';') if ip != '' and ip != '0']
+        else:
+            ips.append(str(i))

Review Comment:
   This is just me, but I would try to check if it's a valid IPv4/IPv6 here 
prior to adding it to the list. Not check the list afterwards, but that's just 
me.
   
   If it's valid, add the IP object to the list



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to