[Openvpn-devel] [L] Change in openvpn[master]: Remove openvpn_snprintf and similar functions
Attention is currently required from: cron2, ordex, plaisthos. flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/547?usp=email ) Change subject: Remove openvpn_snprintf and similar functions .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Gerrit-Change-Number: 547 Gerrit-PatchSet: 5 Gerrit-Owner: plaisthos Gerrit-Reviewer: cron2 Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: ordex Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-Attention: plaisthos Gerrit-Attention: cron2 Gerrit-Attention: ordex Gerrit-Comment-Date: Mon, 06 May 2024 10:18:07 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [L] Change in openvpn[master]: Remove openvpn_snprintf and similar functions
Attention is currently required from: cron2, ordex, plaisthos. flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/547?usp=email ) Change subject: Remove openvpn_snprintf and similar functions .. Patch Set 5: (1 comment) File src/openvpn/proxy.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/1cc5661d_bc0d1f81 : PS4, Line 966: > What do you mean with spurious? It changes openvpn_snprintf to snprintf Sorry, seems I was too brief. I meant the empty line you inserted here. Either way, not important. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Gerrit-Change-Number: 547 Gerrit-PatchSet: 5 Gerrit-Owner: plaisthos Gerrit-Reviewer: cron2 Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: ordex Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-Attention: plaisthos Gerrit-Attention: cron2 Gerrit-Attention: ordex Gerrit-Comment-Date: Mon, 06 May 2024 10:00:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: plaisthos Comment-In-Reply-To: flichtenheld Gerrit-MessageType: comment ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [L] Change in openvpn[master]: Remove openvpn_snprintf and similar functions
Attention is currently required from: cron2, flichtenheld, ordex. plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/547?usp=email ) Change subject: Remove openvpn_snprintf and similar functions .. Patch Set 5: (1 comment) File tests/unit_tests/openvpn/test_buffer.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/4a0f4211_d038f0d1 : PS4, Line 401: #pragma GCC diagnostic pop > yeah, although it seems work this way as well. Will fix. Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Gerrit-Change-Number: 547 Gerrit-PatchSet: 5 Gerrit-Owner: plaisthos Gerrit-Reviewer: cron2 Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: ordex Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-Attention: cron2 Gerrit-Attention: flichtenheld Gerrit-Attention: ordex Gerrit-Comment-Date: Fri, 03 May 2024 16:48:06 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: plaisthos Comment-In-Reply-To: flichtenheld Gerrit-MessageType: comment ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [L] Change in openvpn[master]: Remove openvpn_snprintf and similar functions
Attention is currently required from: cron2, flichtenheld, ordex. plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/547?usp=email ) Change subject: Remove openvpn_snprintf and similar functions .. Patch Set 4: (2 comments) File src/openvpn/proxy.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/aac61c1c_be6f9120 : PS4, Line 966: > Why this spurious change? What do you mean with spurious? It changes openvpn_snprintf to snprintf File tests/unit_tests/openvpn/test_buffer.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/e325e787_1f2bea7f : PS4, Line 401: #pragma GCC diagnostic pop > Should that be "clang" instead of "GCC"? yeah, although it seems work this way as well. Will fix. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Gerrit-Change-Number: 547 Gerrit-PatchSet: 4 Gerrit-Owner: plaisthos Gerrit-Reviewer: cron2 Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: ordex Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-Attention: cron2 Gerrit-Attention: flichtenheld Gerrit-Attention: ordex Gerrit-Comment-Date: Fri, 03 May 2024 16:44:39 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: flichtenheld Gerrit-MessageType: comment ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [L] Change in openvpn[master]: Remove openvpn_snprintf and similar functions
Attention is currently required from: cron2, ordex, plaisthos. flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/547?usp=email ) Change subject: Remove openvpn_snprintf and similar functions .. Patch Set 4: Code-Review-1 (2 comments) File src/openvpn/proxy.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/44a4b5a4_ab3ddf72 : PS4, Line 966: Why this spurious change? File tests/unit_tests/openvpn/test_buffer.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/9f564a6c_9986bda3 : PS4, Line 401: #pragma GCC diagnostic pop Should that be "clang" instead of "GCC"? -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Gerrit-Change-Number: 547 Gerrit-PatchSet: 4 Gerrit-Owner: plaisthos Gerrit-Reviewer: cron2 Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: ordex Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-Attention: plaisthos Gerrit-Attention: cron2 Gerrit-Attention: ordex Gerrit-Comment-Date: Fri, 03 May 2024 15:49:05 + 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
[Openvpn-devel] [L] Change in openvpn[master]: Remove openvpn_snprintf and similar functions
Attention is currently required from: cron2, flichtenheld, ordex. plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/547?usp=email ) Change subject: Remove openvpn_snprintf and similar functions .. Patch Set 3: (2 comments) File src/openvpnserv/interactive.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/a795b02d_09b66782 : PS2, Line 2007: sud.options, svc_pipe); > This makes GHA msbuild build fails now, with […] Done File tests/unit_tests/openvpn/test_buffer.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/f9701207_7c4b904b : PS2, Line 369: * for this unit test. We know that are doing this that are truncated > I think there is some typ0 here. […] Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Gerrit-Change-Number: 547 Gerrit-PatchSet: 3 Gerrit-Owner: plaisthos Gerrit-Reviewer: cron2 Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: ordex Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-Attention: cron2 Gerrit-Attention: flichtenheld Gerrit-Attention: ordex Gerrit-Comment-Date: Thu, 02 May 2024 17:44:31 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: cron2 Comment-In-Reply-To: ordex Gerrit-MessageType: comment ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [L] Change in openvpn[master]: Remove openvpn_snprintf and similar functions
Attention is currently required from: flichtenheld, plaisthos. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/547?usp=email ) Change subject: Remove openvpn_snprintf and similar functions .. Patch Set 2: Code-Review-1 (3 comments) Patchset: PS2: Unfortunately, this fails -Werror msbuild builds now, see comment in interactive.c File src/openvpn/proxy.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/b37effa6_d450b13a : PS2, Line 962: if (sret >= sizeof(buf)) > if this can truly happen, does it mean that the buffer is undersized compared > to the size of all var […] this code is not part of the actual patchset, but a gerrit artefact due to rebasing. *This* patchset only replaces openvpn_snprintf() with snprintf(). File src/openvpnserv/interactive.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/918c9772_0150ba41 : PS2, Line 2007: sud.options, svc_pipe); This makes GHA msbuild build fails now, with ``` D:\a\openvpn\openvpn\src\openvpnserv\interactive.c(2006,37): warning C4477: 'swprintf' : format string '%llu' requires an argument of type 'unsigned __int64', but variadic argument 2 has type 'HANDLE' [D:\a\openvpn\openvpn\out\build\win-amd64-release\src\openvpnserv\openvpnserv.vcxproj] ``` so it seems the cast to something-int (`DWORD`?) is needed. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Gerrit-Change-Number: 547 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos Gerrit-Reviewer: cron2 Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: ordex Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-Attention: plaisthos Gerrit-Attention: flichtenheld Gerrit-Comment-Date: Mon, 08 Apr 2024 08:22:27 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: ordex Gerrit-MessageType: comment ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [L] Change in openvpn[master]: Remove openvpn_snprintf and similar functions
Attention is currently required from: flichtenheld, plaisthos. ordex has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/547?usp=email ) Change subject: Remove openvpn_snprintf and similar functions .. Patch Set 2: Code-Review+2 (4 comments) Patchset: PS2: closing some comments as they were commenting code that did not really change with this patch... The only remaining comment is about a comment in the UT which is a bit cryptic. Maybe that can be adjusted on the fly. Other than that, the code looks good to me <3 File src/openvpn/proxy.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/f78adfbf_ca231ac7 : PS2, Line 962: if (sret >= sizeof(buf)) > if this can truly happen, does it mean that the buffer is undersized compared > to the size of all var […] Done File src/openvpn/socks.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/0f6508ea_b21108c0 : PS2, Line 114: (int) strlen(creds.username), creds.username, > normally we don't put a paceb etween the cast and the variable name. […] Done http://gerrit.openvpn.net/c/openvpn/+/547/comment/b4ed3e45_e9af3876 : PS2, Line 116: ASSERT(sret <= sizeof(to_send)); > why ASSERT here while in other cases we just go to error or cleanup? Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Gerrit-Change-Number: 547 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: ordex Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-Attention: plaisthos Gerrit-Attention: flichtenheld Gerrit-Comment-Date: Mon, 08 Apr 2024 06:59:00 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: ordex Gerrit-MessageType: comment ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [L] Change in openvpn[master]: Remove openvpn_snprintf and similar functions
Attention is currently required from: flichtenheld, plaisthos. ordex has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/547?usp=email ) Change subject: Remove openvpn_snprintf and similar functions .. Patch Set 2: (5 comments) Patchset: PS2: some nit picks below.. File src/openvpn/proxy.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/d2c89116_22dbc48e : PS2, Line 962: if (sret >= sizeof(buf)) if this can truly happen, does it mean that the buffer is undersized compared to the size of all variables we are putting together? Therefore, wouldn't it make more sense to extend the size of the buffer to ensure that no matter what we save in those variables, we will always be able to create the HTTP header? Or there is a limit with the HTTP header that we have to deal with? My concern is that we are not preventing people from filling those variables as they please, but we will then fail to put them together for no good reason. does it make sense? File src/openvpn/socks.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/8bdf0e3c_8356c931 : PS2, Line 114: (int) strlen(creds.username), creds.username, normally we don't put a paceb etween the cast and the variable name. This comments applies to all other casts below http://gerrit.openvpn.net/c/openvpn/+/547/comment/b417986e_a5f333df : PS2, Line 116: ASSERT(sret <= sizeof(to_send)); why ASSERT here while in other cases we just go to error or cleanup? File tests/unit_tests/openvpn/test_buffer.c: http://gerrit.openvpn.net/c/openvpn/+/547/comment/f8c8a505_dbfb9729 : PS2, Line 369: * for this unit test. We know that are doing this that are truncated I think there is some typ0 here. Maybe something like: "We know that results will be truncated and we actually want to test that". -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Gerrit-Change-Number: 547 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: ordex Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-Attention: plaisthos Gerrit-Attention: flichtenheld Gerrit-Comment-Date: Wed, 27 Mar 2024 10:48:48 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [L] Change in openvpn[master]: Remove openvpn_snprintf and similar functions
Attention is currently required from: flichtenheld. plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/547?usp=email ) Change subject: Remove openvpn_snprintf and similar functions .. Patch Set 1: Code-Review-1 (1 comment) Patchset: PS1: not -Werror clean yet -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Gerrit-Change-Number: 547 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: ordex Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-Attention: flichtenheld Gerrit-Comment-Date: Mon, 25 Mar 2024 11:14:07 + 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
[Openvpn-devel] [L] Change in openvpn[master]: Remove openvpn_snprintf and similar functions
Attention is currently required from: flichtenheld, plaisthos. ordex has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/547?usp=email ) Change subject: Remove openvpn_snprintf and similar functions .. Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: as far as I understand we can't restore the broken behavior unless we specify that macro or we switch to __snprintf. Both can't happen accidentally, therefore it's not possible introduce the buggy behavior by mistake. In the worst case the code won't compile on old MS systems (pre-VS2015/VC14). -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac Gerrit-Change-Number: 547 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: ordex Gerrit-CC: openvpn-devel Gerrit-Attention: plaisthos Gerrit-Attention: flichtenheld Gerrit-Comment-Date: Mon, 25 Mar 2024 01:15:22 + 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
