Attention is currently required from: plaisthos, ralf_lici. flichtenheld has posted comments on this change by ralf_lici. ( http://gerrit.openvpn.net/c/openvpn/+/739?usp=email )
Change subject: options: add IPv4 CIDR parsing for relevant directives ...................................................................... Patch Set 9: Code-Review-1 (5 comments) File doc/man-sections/server-options.rst: http://gerrit.openvpn.net/c/openvpn/+/739/comment/6c774407_c41b8c34?usp=email : PS9, Line 309: view. I would find it a bit easier to read if this was more explicit. Maybe add something like "So the server will actually push ``ifconfig alias remote/netmask`` to the client"? File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/739/comment/35d57942_8451aec7?usp=email : PS9, Line 7288: const int lev = M_WARN; Please fix to msglvl_t http://gerrit.openvpn.net/c/openvpn/+/739/comment/9c9d016a_9cface99?usp=email : PS9, Line 7688: remote_netmask = getaddr(GETADDR_HOST_ORDER | GETADDR_RESOLVE, ifconfig_push_parms[2], 0, A bit absurd that we need to parse netmask here again if we generated it before. But probably not worth trying to optimize that away. File src/openvpn/options_util.c: http://gerrit.openvpn.net/c/openvpn/+/739/comment/8d1749e3_5a891d4c?usp=email : PS9, Line 232: * socket_util.c dependencies into options_util unit-test targets. */ This feels like a weak justification to duplicate code. File tests/unit_tests/openvpn/test_options_parse.c: http://gerrit.openvpn.net/c/openvpn/+/739/comment/ab0e93b7_3d53fbba?usp=email : PS9, Line 330: test_convert_ipv4_cidr_parms_coverage(void **state) This does not feel like it provides actual "coverage". The examples are all too similar. To get coverage you should test a bigger variety of "bits" (maybe throw a "0" and a "32" in there at least?) and more interesting variants of input parameters (even if they are not currently realistic). E.g. use minimal and maximal values for network_idx and max_idx. Also include the case where the network parameter is a DNS name -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/739?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Iae04ad8715e40dfc76475c2c5b9a766c9604efc9 Gerrit-Change-Number: 739 Gerrit-PatchSet: 9 Gerrit-Owner: ralf_lici <[email protected]> Gerrit-Reviewer: flichtenheld <[email protected]> Gerrit-Reviewer: plaisthos <[email protected]> Gerrit-CC: openvpn-devel <[email protected]> Gerrit-Attention: plaisthos <[email protected]> Gerrit-Attention: ralf_lici <[email protected]> Gerrit-Comment-Date: Mon, 09 Mar 2026 13:10:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
