|
From: flichtenheld (C. Review) <ge...@op...> - 2025-12-05 14:48:43
|
Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1417?usp=email
to review the following change.
Change subject: CMake: For VS build, document what we're missing from /W3
......................................................................
CMake: For VS build, document what we're missing from /W3
Basically -Wconversion and -Wsign-compare, so as expected.
Change-Id: Iffc114939cb37129057e9c4864fae9e09c3c7fe4
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M CMakeLists.txt
1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/17/1417/1
diff --git a/CMakeLists.txt b/CMakeLists.txt
index b3142e4..906fa04 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -85,9 +85,12 @@
if (USE_WERROR)
add_compile_options(/WX)
endif ()
+ # C4018: signed/unsigned mismatch
+ # C4244: conversion from 'type1' to 'type2', possible loss of data
+ # C4267: conversion from 'size_t' to 'type', possible loss of data
add_compile_options(
/MP
- /W2
+ /W3 /wd4018 /wd4267 /wd4244
/sdl
/Qspectre
/guard:cf
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1417?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iffc114939cb37129057e9c4864fae9e09c3c7fe4
Gerrit-Change-Number: 1417
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-12-05 16:11:16
|
Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1417?usp=email
to look at the new patch set (#2).
Change subject: CMake: For VS build, document what we're missing from /W3
......................................................................
CMake: For VS build, document what we're missing from /W3
Basically -Wconversion and -Wsign-compare, so as expected.
Github: #382
Change-Id: Iffc114939cb37129057e9c4864fae9e09c3c7fe4
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M CMakeLists.txt
1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/17/1417/2
diff --git a/CMakeLists.txt b/CMakeLists.txt
index b3142e4..906fa04 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -85,9 +85,12 @@
if (USE_WERROR)
add_compile_options(/WX)
endif ()
+ # C4018: signed/unsigned mismatch
+ # C4244: conversion from 'type1' to 'type2', possible loss of data
+ # C4267: conversion from 'size_t' to 'type', possible loss of data
add_compile_options(
/MP
- /W2
+ /W3 /wd4018 /wd4267 /wd4244
/sdl
/Qspectre
/guard:cf
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1417?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iffc114939cb37129057e9c4864fae9e09c3c7fe4
Gerrit-Change-Number: 1417
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
|
|
From: flichtenheld (C. Review) <ge...@op...> - 2025-12-05 17:58:04
|
Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1417?usp=email
to look at the new patch set (#3).
Change subject: CMake: For VS build, switch from /W2 to /W3
......................................................................
CMake: For VS build, switch from /W2 to /W3
But exclude the added checks that currently have failures
so that we can keep /WX enabled.
Basically this excludes -Wconversion and -Wsign-compare,
as expected from our GCC/Clang flags.
Github: #382
Change-Id: Iffc114939cb37129057e9c4864fae9e09c3c7fe4
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M CMakeLists.txt
1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/17/1417/3
diff --git a/CMakeLists.txt b/CMakeLists.txt
index b3142e4..906fa04 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -85,9 +85,12 @@
if (USE_WERROR)
add_compile_options(/WX)
endif ()
+ # C4018: signed/unsigned mismatch
+ # C4244: conversion from 'type1' to 'type2', possible loss of data
+ # C4267: conversion from 'size_t' to 'type', possible loss of data
add_compile_options(
/MP
- /W2
+ /W3 /wd4018 /wd4267 /wd4244
/sdl
/Qspectre
/guard:cf
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1417?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iffc114939cb37129057e9c4864fae9e09c3c7fe4
Gerrit-Change-Number: 1417
Gerrit-PatchSet: 3
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-12-08 11:34:52
|
Attention is currently required from: flichtenheld, plaisthos. cron2 has posted comments on this change by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1417?usp=email ) Change subject: CMake: For VS build, switch from /W2 to /W3 ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1417?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: Iffc114939cb37129057e9c4864fae9e09c3c7fe4 Gerrit-Change-Number: 1417 Gerrit-PatchSet: 3 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Mon, 08 Dec 2025 11:34:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes |
|
From: Gert D. <ge...@gr...> - 2025-12-08 11:36:49
|
From: Frank Lichtenheld <fr...@li...> But exclude the added checks that currently have failures so that we can keep /WX enabled. Basically this excludes -Wconversion and -Wsign-compare, as expected from our GCC/Clang flags. Github: #382 Change-Id: Iffc114939cb37129057e9c4864fae9e09c3c7fe4 Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1417 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1417 This mail reflects revision 3 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <ge...@gr...> diff --git a/CMakeLists.txt b/CMakeLists.txt index b3142e4..906fa04 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -85,9 +85,12 @@ if (USE_WERROR) add_compile_options(/WX) endif () + # C4018: signed/unsigned mismatch + # C4244: conversion from 'type1' to 'type2', possible loss of data + # C4267: conversion from 'size_t' to 'type', possible loss of data add_compile_options( /MP - /W2 + /W3 /wd4018 /wd4267 /wd4244 /sdl /Qspectre /guard:cf |
|
From: Gert D. <ge...@gr...> - 2025-12-08 14:41:56
|
Windows building, turn on more warnings (welcome), ignoring those that
we do not have fixed yet - makes sense :-) - plus documentation!
Tested via GHA builds.
Your patch has been applied to the master branch.
commit 9a2420fd63265166d1b60a38e8180aa360473ddc
Author: Frank Lichtenheld
Date: Mon Dec 8 12:36:30 2025 +0100
CMake: For VS build, switch from /W2 to /W3
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1417
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg34876.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-12-08 14:41:57
|
cron2 has uploaded a new patch set (#4) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1417?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: CMake: For VS build, switch from /W2 to /W3 ...................................................................... CMake: For VS build, switch from /W2 to /W3 But exclude the added checks that currently have failures so that we can keep /WX enabled. Basically this excludes -Wconversion and -Wsign-compare, as expected from our GCC/Clang flags. Github: #382 Change-Id: Iffc114939cb37129057e9c4864fae9e09c3c7fe4 Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1417 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg34876.html Signed-off-by: Gert Doering <ge...@gr...> --- M CMakeLists.txt 1 file changed, 4 insertions(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/17/1417/4 diff --git a/CMakeLists.txt b/CMakeLists.txt index b3142e4..906fa04 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -85,9 +85,12 @@ if (USE_WERROR) add_compile_options(/WX) endif () + # C4018: signed/unsigned mismatch + # C4244: conversion from 'type1' to 'type2', possible loss of data + # C4267: conversion from 'size_t' to 'type', possible loss of data add_compile_options( /MP - /W2 + /W3 /wd4018 /wd4267 /wd4244 /sdl /Qspectre /guard:cf -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1417?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Iffc114939cb37129057e9c4864fae9e09c3c7fe4 Gerrit-Change-Number: 1417 Gerrit-PatchSet: 4 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
|
From: cron2 (C. Review) <ge...@op...> - 2025-12-08 14:42:04
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1417?usp=email ) Change subject: CMake: For VS build, switch from /W2 to /W3 ...................................................................... CMake: For VS build, switch from /W2 to /W3 But exclude the added checks that currently have failures so that we can keep /WX enabled. Basically this excludes -Wconversion and -Wsign-compare, as expected from our GCC/Clang flags. Github: #382 Change-Id: Iffc114939cb37129057e9c4864fae9e09c3c7fe4 Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Gert Doering <ge...@gr...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1417 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg34876.html Signed-off-by: Gert Doering <ge...@gr...> --- M CMakeLists.txt 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b3142e4..906fa04 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -85,9 +85,12 @@ if (USE_WERROR) add_compile_options(/WX) endif () + # C4018: signed/unsigned mismatch + # C4244: conversion from 'type1' to 'type2', possible loss of data + # C4267: conversion from 'size_t' to 'type', possible loss of data add_compile_options( /MP - /W2 + /W3 /wd4018 /wd4267 /wd4244 /sdl /Qspectre /guard:cf -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1417?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Iffc114939cb37129057e9c4864fae9e09c3c7fe4 Gerrit-Change-Number: 1417 Gerrit-PatchSet: 4 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |