| Message ID | 1540306288-23847-1-git-send-email-lstipakov@gmail.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [Openvpn-devel] Wrap openvpn_swprintf into Windows define | expand |
Hi, On 23/10/18 22:51, Lev Stipakov wrote: > From: Lev Stipakov <lev@openvpn.net> > > Commit 43a5a4f3b4e411419639c195fee8a76495fdc88e added > vswprintf() call which turned to me missing in OpenBSD 4.9. > > Since that call is inside openvpn_swprintf() function which > is only used by Windows, wrap that function info #ifdef _WIN32. > > Signed-off-by: Lev Stipakov <lev@openvpn.net> This patch introduces another ifdef, but it is enclosing a whole function to prevent it from being compiled on non-windows platforms, therefore I consider it "acceptable" for the time being. This patch fixes building openvpn on OpenBSD <= 4.9 (maybe this info should be added to the commit message). Other than that: Acked-by: Antonio Quartulli <antonio@openvpn.net>
On Wed, Oct 24, 2018 at 6:23 AM Antonio Quartulli <a@unstable.cc> wrote: > > Hi, > > On 23/10/18 22:51, Lev Stipakov wrote: > > From: Lev Stipakov <lev@openvpn.net> > > > > Commit 43a5a4f3b4e411419639c195fee8a76495fdc88e added > > vswprintf() call which turned to me missing in OpenBSD 4.9. > > > > Since that call is inside openvpn_swprintf() function which > > is only used by Windows, wrap that function info #ifdef _WIN32. > > > > Signed-off-by: Lev Stipakov <lev@openvpn.net> > > This patch introduces another ifdef, but it is enclosing a whole > function to prevent it from being compiled on non-windows platforms, > therefore I consider it "acceptable" for the time being. > > This patch fixes building openvpn on OpenBSD <= 4.9 (maybe this info > should be added to the commit message). Also there is a misplaced and mismatched comment added to buffer.c that says + /* vswprintf is missing in OpenBSD 4.2 */ len = vswprintf(str, size, format, arglist); Makes little sense in that context and why 4.2.. That one could be removed. Otherwise an ACK from me too. Selva
On 24/10/18 14:39, Selva Nair wrote: > On Wed, Oct 24, 2018 at 6:23 AM Antonio Quartulli <a@unstable.cc> wrote: >> >> Hi, >> >> On 23/10/18 22:51, Lev Stipakov wrote: >>> From: Lev Stipakov <lev@openvpn.net> >>> >>> Commit 43a5a4f3b4e411419639c195fee8a76495fdc88e added >>> vswprintf() call which turned to me missing in OpenBSD 4.9. >>> >>> Since that call is inside openvpn_swprintf() function which >>> is only used by Windows, wrap that function info #ifdef _WIN32. >>> >>> Signed-off-by: Lev Stipakov <lev@openvpn.net> >> >> This patch introduces another ifdef, but it is enclosing a whole >> function to prevent it from being compiled on non-windows platforms, >> therefore I consider it "acceptable" for the time being. >> >> This patch fixes building openvpn on OpenBSD <= 4.9 (maybe this info >> should be added to the commit message). > > Also there is a misplaced and mismatched comment added to > buffer.c that says > > + /* vswprintf is missing in OpenBSD 4.2 */ > len = vswprintf(str, size, format, arglist); > > Makes little sense in that context and why 4.2.. > That one could be removed. I'd say we should have such a reference in right below the #ifdef _WIN32 a few lines higher up. And it could be slightly more informative (and should reference OpenBSD 4.9, which is the distro exploding without this patch. Primary reason for this message is to explain why we do not provide this function to "everyone", if someone has a use case for it later on in a non-Windows code path. If this is needed elsewhere, we at least know instantly what might break and we can evaluate if it's worth keeping OpenBSD 4.9 support. I suggest the comment below the #ifdef _WIN32 to be something like: /* * openvpn_swprintf() is currently only used by Windows code paths * and when enabled for all platforms it will currently break * OpenBSD 4.9 which lacks vswprintf(3) support in its libc. */
Hi, On Wed, Oct 24, 2018 at 9:00 AM David Sommerseth <openvpn@sf.lists.topphemmelig.net> wrote: > > On 24/10/18 14:39, Selva Nair wrote: > > On Wed, Oct 24, 2018 at 6:23 AM Antonio Quartulli <a@unstable.cc> wrote: > >> > >> Hi, > >> > > > > > > Also there is a misplaced and mismatched comment added to > > buffer.c that says > > > > + /* vswprintf is missing in OpenBSD 4.2 */ > > len = vswprintf(str, size, format, arglist); > > > > Makes little sense in that context and why 4.2.. > > That one could be removed. > I'd say we should have such a reference in right below the #ifdef _WIN32 a few > lines higher up. And it could be slightly more informative (and should > reference OpenBSD 4.9, which is the distro exploding without this patch. Yes, that's why I wrote "misplaced" and "mismatched" :) > I suggest the comment below the #ifdef _WIN32 to be something like: > > /* > * openvpn_swprintf() is currently only used by Windows code paths > * and when enabled for all platforms it will currently break > * OpenBSD 4.9 which lacks vswprintf(3) support in its libc. > */ Looks good except, <=4.9, not just 4.9, or whatever version is appropriate. (As Antonio already pointed out.) Selva
Your patch has been applied to the master branch.
Make OpenBSD great^w green again! (Tested before pushing :) )
(I have amended the comments according to the discussion)
commit 4ce1a9b6b3ae0be32a3d999c5bc93731c64d7bbb
Author: Lev Stipakov
Date: Tue Oct 23 17:51:28 2018 +0300
Wrap openvpn_swprintf into Windows define
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <1540306288-23847-1-git-send-email-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17799.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 8ca189f..acf76ae 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -310,6 +310,7 @@ openvpn_snprintf(char *str, size_t size, const char *format, ...) return (len >= 0 && len < size); } +#ifdef _WIN32 bool openvpn_swprintf(wchar_t *const str, const size_t size, const wchar_t *const format, ...) { @@ -318,12 +319,14 @@ openvpn_swprintf(wchar_t *const str, const size_t size, const wchar_t *const for if (size > 0) { va_start(arglist, format); + /* vswprintf is missing in OpenBSD 4.2 */ len = vswprintf(str, size, format, arglist); va_end(arglist); str[size - 1] = L'\0'; } return (len >= 0 && len < size); } +#endif /* * write a string to the end of a buffer that was diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h index c5b78a0..c48c925 100644 --- a/src/openvpn/buffer.h +++ b/src/openvpn/buffer.h @@ -449,7 +449,11 @@ __attribute__ ((format(__printf__, 3, 4))) ; +#ifdef _WIN32 /* + * This is under #ifdef because only Windows-specific code + * uses this function and its implementation is broken in OpenBSD 4.9 + * * Like swprintf but guarantees null termination for size > 0 */ bool @@ -458,6 +462,7 @@ openvpn_swprintf(wchar_t *const str, const size_t size, const wchar_t *const for * Unlike in openvpn_snprintf, we cannot use format attributes since * GCC doesn't support wprintf as archetype. */ +#endif /* * remove/add trailing characters