Message ID | 20211015175330.15760-1-selva.nair@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | [Openvpn-devel] Fix some more wrong defines in config-msvc.h | expand |
On Fri, Oct 15, 2021 at 1:53 PM <selva.nair@gmail.com> wrote: > From: Selva Nair <selva.nair@gmail.com> > > Not sure where these came from, but here goes: > > S_IRUSR = 0 --> _S_IREAD > S_IWUSR = 0 --> _S_IWRITE > > ENABLE_DEBUG is on, but I do not think we want it in production build. > Changed to ENABLE_DEBUG 0 > > S_IRGRP is not defined but seems to be used. I have added it, remove if > not required. > This define is based on mingw and matches MS docs on <filesystem> > ( > https://docs.microsoft.com/en-us/cpp/standard-library/filesystem-enumerations?view=msvc-160 > ) > > Should fix Trac #1430 but untested (I do not have a setup at hand for msvc > build). > The patch for master doesn't apply cleantly to 2.5. Please use the next one marked "for 2.5" on the release branch. Selva <div dir="ltr"><div dir="ltr"><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Oct 15, 2021 at 1:53 PM <<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Selva Nair <<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>><br> <br> Not sure where these came from, but here goes:<br> <br> S_IRUSR = 0 --> _S_IREAD<br> S_IWUSR = 0 --> _S_IWRITE<br> <br> ENABLE_DEBUG is on, but I do not think we want it in production build.<br> Changed to ENABLE_DEBUG 0<br> <br> S_IRGRP is not defined but seems to be used. I have added it, remove if<br> not required.<br> This define is based on mingw and matches MS docs on <filesystem><br> (<a href="https://docs.microsoft.com/en-us/cpp/standard-library/filesystem-enumerations?view=msvc-160" rel="noreferrer" target="_blank">https://docs.microsoft.com/en-us/cpp/standard-library/filesystem-enumerations?view=msvc-160</a>)<br> <br> Should fix Trac #1430 but untested (I do not have a setup at hand for msvc build).<br></blockquote><div><br></div><div>The patch for master doesn't apply cleantly to 2.5. Please use the next one marked "for 2.5" on the release branch.<div><br></div><div>Selva</div></div></div></div>
Hi, > Not sure where these came from, but here goes: According to git blame, those dates back to 2012: https://github.com/OpenVPN/openvpn/commit/4b1a82db0975088dbfe69c2a97f0a96ef972b2a4#diff-f17c6f8c8df2b2934b574e407cbde107338969c6898ce8e8965dc289ae979fc6R97 > S_IRUSR = 0 --> _S_IREAD > S_IWUSR = 0 --> _S_IWRITE MSFT docs says that: "If a value other than some combination of _S_IREAD and _S_IWRITE is specified for pmode—even if it would specify a valid pmode in another operating system—or if any value other than the allowed oflag values is specified, the function generates an assertion in Debug mode and invokes the invalid parameter handler, as described in Parameter Validation." I tried Debug build with --status but haven't seen any assertions. Anyway, it is clear that passing 0 was incorrect. > Should fix Trac #1430 but untested (I do not have a setup at hand for msvc build). If you push your branch to GitHub, it will kick off GitHub Actions build which produces MSVC-built artifacts. Built and tested locally, works as expected - status file is created without "readonly" attribute. Acked-by: Lev Stipakov <lstipakov@gmail.com>
I am sorry, I need to take the ack back. Probably cron2 can fix it in place? -#define ENABLE_DEBUG 1 +#define ENABLE_DEBUG 0 This doesn't really undef ENABLE_DEBUG. Can we just remove "-#define ENABLE_DEBUG 1" ? -- -Lev
Thanks. As with the 2.5 patch, removed the DEBUG #define, and changed commit message to just refer to trac with "Trac: #1430" Your patch has been applied to the master branch. commit 077445d0d82128dc90e50043546d4a2d8647eb9c Author: Selva Nair Date: Fri Oct 15 13:53:30 2021 -0400 Fix some more wrong defines in config-msvc.h Acked-by: Lev Stipakov <lstipakov@gmail.com> Message-Id: <20211015175330.15760-1-selva.nair@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22942.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/config-msvc.h b/config-msvc.h index b28d8742..301e7ebc 100644 --- a/config-msvc.h +++ b/config-msvc.h @@ -4,7 +4,7 @@ #define ENABLE_PF 1 #define ENABLE_CRYPTO_OPENSSL 1 -#define ENABLE_DEBUG 1 +#define ENABLE_DEBUG 0 #define ENABLE_FRAGMENT 1 #define ENABLE_HTTP_PROXY 1 #define ENABLE_LZO 1 @@ -67,8 +67,9 @@ #define in_addr_t uint32_t #define ssize_t SSIZE_T -#define S_IRUSR 0 -#define S_IWUSR 0 +#define S_IRUSR _S_IREAD +#define S_IWUSR _S_IWRITE +#define S_IRGRP (S_IRUSR >> 3) #define R_OK 4 #define W_OK 2 #define X_OK 1
From: Selva Nair <selva.nair@gmail.com> Not sure where these came from, but here goes: S_IRUSR = 0 --> _S_IREAD S_IWUSR = 0 --> _S_IWRITE ENABLE_DEBUG is on, but I do not think we want it in production build. Changed to ENABLE_DEBUG 0 S_IRGRP is not defined but seems to be used. I have added it, remove if not required. This define is based on mingw and matches MS docs on <filesystem> (https://docs.microsoft.com/en-us/cpp/standard-library/filesystem-enumerations?view=msvc-160) Should fix Trac #1430 but untested (I do not have a setup at hand for msvc build). --- config-msvc.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)