Attention is currently required from: flichtenheld, kallisti5, plaisthos. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/688?usp=email )
Change subject: Haiku: Add calls to manage routing table ...................................................................... Patch Set 8: Code-Review-1 (10 comments) Patchset: PS8: The code should work, technically, but since we're in a review cycle, I've found more things that should be changed to confirm to our style guide (or to just not copy existing ugliness). We do have an uncrustify config in the source tree to get the whitespace auto-fixed (dev-tools/uncrustify.conf), but you need a slightly older uncrustify version (0.72) to actually produce correct output. File src/openvpn/route.c: http://gerrit.openvpn.net/c/openvpn/+/688/comment/2d7989ed_31075357 : PS8, Line 1858: { please remove this indentation level, it looks unnecessary. (We used to need this before we went for C99 compatibility, and declaring local variables did not work unless we had a local {} block - but nowadays, this is no longer needed - it's just that nobody else fixed the other code blocks yet) http://gerrit.openvpn.net/c/openvpn/+/688/comment/a5af820d_fdaa953c : PS8, Line 2350: { same here http://gerrit.openvpn.net/c/openvpn/+/688/comment/631992d6_e721d0c9 : PS8, Line 2543: { same here. Also: I see route *deletion* for IPv6 routes, but no route *addition*, and also no ifconfig for IPv6 (other patch). So maybe IPv6 route deletion should go to a followup patch "add IPv6 support for Haiku" which has ifconfig + route add + route delete http://gerrit.openvpn.net/c/openvpn/+/688/comment/3f9c168e_262eae5a : PS8, Line 3940: if (sockfd < 0) { this is a style guide violation - normally, for first time submitters, I'd fix that on the fly but since we're here anyway... we require the opening brace to be on its own line ``` if (thing) { /* do something */ } ``` always, no exceptions. http://gerrit.openvpn.net/c/openvpn/+/688/comment/509a08d9_9b43b7cf : PS8, Line 3947: if (ioctl(sockfd, SIOCGRTSIZE, &config, sizeof(struct ifconf)) < 0) { this http://gerrit.openvpn.net/c/openvpn/+/688/comment/2af50a79_267b9f62 : PS8, Line 3954: return; we require to always have braces, even if it's a single statement ``` if (size == 0) { return; } ``` http://gerrit.openvpn.net/c/openvpn/+/688/comment/2f4ff9d8_38d0a4d4 : PS8, Line 3961: if (ioctl(sockfd, SIOCGRTTABLE, &config, sizeof(struct ifconf)) < 0) { bracket http://gerrit.openvpn.net/c/openvpn/+/688/comment/54c49fc8_a46d3299 : PS8, Line 3969: while (interface < end) { bracket http://gerrit.openvpn.net/c/openvpn/+/688/comment/ce646a6b_df5cd804 : PS8, Line 3978: if (route.destination != NULL) these need braces... -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/688?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I1a22724f28c5cd47f6df178b49f44087d7c2b6fd Gerrit-Change-Number: 688 Gerrit-PatchSet: 8 Gerrit-Owner: kallisti5 <a...@terarocket.io> Gerrit-Reviewer: cron2 <g...@greenie.muc.de> Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com> Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org> Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net> Gerrit-Attention: plaisthos <arne-open...@rfc2549.org> Gerrit-Attention: flichtenheld <fr...@lichtenheld.com> Gerrit-Attention: kallisti5 <a...@terarocket.io> Gerrit-Comment-Date: Wed, 27 Nov 2024 10:52:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
