| Message ID | 20171010231130.6832-5-simon@rozman.si |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel,01/13] snwprintf() => _snwprintf() | expand |
Hi, Thanks for these patches. On Tue, Oct 10, 2017 at 7:11 PM, <simon@rozman.si> wrote: > From: Simon Rozman <simon@rozman.si> > > Note: NETIOAPI_API is defined as: > #define NETIOAPI_API NETIO_STATUS NETIOAPI_API_ > #define NETIOAPI_API_ WINAPI > > I am not sure whether interactive.c will compile using mingw with this > patch. If not: > 1. We can introduce some > #ifdef _MSC_VER > #else > #endif > 2. Since NTDDI_VERSION=NTDDI_VISTA, most of the dynamic API loading by > GetProcAddress() could be simplified to statically linked API calls, > avoiding function prototype declarations altogether. I suggest we do this > anyway to clean-up the code in the future. Yes, those declarations and run-time lookup were slated for removal but somehow never happened. Its a good idea to get rid of those -- no point in tweaking them to suit MSVC. IIRC, at least the versions of mingw that I use (gcc 4.9.3 or later) will build with those stripped but I can test again if you replace this patch with one removing those declarations. Selva <div dir="ltr">Hi,<div><div class="gmail_extra"><br></div><div class="gmail_extra">Thanks for these patches.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 10, 2017 at 7:11 PM, <span dir="ltr"><<a href="mailto:simon@rozman.si" target="_blank">simon@rozman.si</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Simon Rozman <<a href="mailto:simon@rozman.si" target="_blank">simon@rozman.si</a>><br> <br> Note: NETIOAPI_API is defined as:<br> #define NETIOAPI_API NETIO_STATUS NETIOAPI_API_<br> #define NETIOAPI_API_ WINAPI<br> <br> I am not sure whether interactive.c will compile using mingw with this patch. If not:<br> 1. We can introduce some<br> #ifdef _MSC_VER<br> #else<br> #endif<br> 2. Since NTDDI_VERSION=NTDDI_VISTA, most of the dynamic API loading by GetProcAddress() could be simplified to statically linked API calls, avoiding function prototype declarations altogether. I suggest we do this anyway to clean-up the code in the future.</blockquote><div><br></div><div>Yes, those declarations and run-time lookup were slated for removal but somehow never happened. Its a good idea to get rid of those -- no point in tweaking them to suit MSVC. </div><div><br></div><div>IIRC, at least the versions of mingw that I use (gcc 4.9.3 or later) will build with those stripped but I can test again if you replace this patch with one removing those declarations.</div><div><br></div><div>Selva</div></div></div></div></div> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, I shall happily rewrite the code to remove those declarations. However, I'll need to check MSDN on a case-by-case basis to replace only API calls that are standard from Vista on. Do you still maintain 2.3 branch for Windows XP? You do know, that replacing GetProcAddr() with statically linked Vista+ API calls would break compatibility with Windows XP. Best regards, Simon From: Selva [mailto:selva.nair@gmail.com] Sent: Wednesday, October 11, 2017 3:21 PM To: Simon Rozman Cc: openvpn-devel@lists.sourceforge.net Subject: Re: [Openvpn-devel] [PATCH 05/13] Function prototypes are declared as "typedef <return type> (<calling convention> *type_name)(<arguments>)" in MSVC. Hi, Thanks for these patches. On Tue, Oct 10, 2017 at 7:11 PM, <simon@rozman.si> wrote: From: Simon Rozman <simon@rozman.si> Note: NETIOAPI_API is defined as: #define NETIOAPI_API NETIO_STATUS NETIOAPI_API_ #define NETIOAPI_API_ WINAPI I am not sure whether interactive.c will compile using mingw with this patch. If not: 1. We can introduce some #ifdef _MSC_VER #else #endif 2. Since NTDDI_VERSION=NTDDI_VISTA, most of the dynamic API loading by GetProcAddress() could be simplified to statically linked API calls, avoiding function prototype declarations altogether. I suggest we do this anyway to clean-up the code in the future. Yes, those declarations and run-time lookup were slated for removal but somehow never happened. Its a good idea to get rid of those -- no point in tweaking them to suit MSVC. IIRC, at least the versions of mingw that I use (gcc 4.9.3 or later) will build with those stripped but I can test again if you replace this patch with one removing those declarations. Selva <html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 14 (filtered medium)"><style><!-- /* Font Definitions */ @font-face {font-family:Calibri; panose-1:2 15 5 2 2 2 4 3 2 4;} @font-face {font-family:Tahoma; panose-1:2 11 6 4 3 5 4 4 2 4;} /* Style Definitions */ p.MsoNormal, li.MsoNormal, div.MsoNormal {margin:0cm; margin-bottom:.0001pt; font-size:12.0pt; font-family:"Times New Roman","serif";} a:link, span.MsoHyperlink {mso-style-priority:99; color:blue; text-decoration:underline;} a:visited, span.MsoHyperlinkFollowed {mso-style-priority:99; color:purple; text-decoration:underline;} code {mso-style-priority:99; font-family:"Courier New";} p.MsoAcetate, li.MsoAcetate, div.MsoAcetate {mso-style-priority:99; mso-style-link:"Balloon Text Char"; margin:0cm; margin-bottom:.0001pt; font-size:8.0pt; font-family:"Tahoma","sans-serif";} p.MsoQuote, li.MsoQuote, div.MsoQuote {mso-style-priority:29; mso-style-link:"Quote Char"; margin:0cm; margin-bottom:.0001pt; font-size:12.0pt; font-family:"Calibri","sans-serif"; font-style:italic;} span.QuoteChar {mso-style-name:"Quote Char"; mso-style-priority:29; mso-style-link:Quote; font-family:"Calibri","sans-serif"; font-style:italic;} span.EmailStyle20 {mso-style-type:personal-reply; font-family:"Calibri","sans-serif"; color:#1F497D;} span.BalloonTextChar {mso-style-name:"Balloon Text Char"; mso-style-priority:99; mso-style-link:"Balloon Text"; font-family:"Tahoma","sans-serif"; mso-fareast-language:SL;} .MsoChpDefault {mso-style-type:export-only; font-family:"Calibri","sans-serif"; mso-fareast-language:EN-US;} @page WordSection1 {size:612.0pt 792.0pt; margin:70.85pt 70.85pt 70.85pt 70.85pt;} div.WordSection1 {page:WordSection1;} --></style><!--[if gte mso 9]><xml> <o:shapedefaults v:ext="edit" spidmax="1026" /> </xml><![endif]--><!--[if gte mso 9]><xml> <o:shapelayout v:ext="edit"> <o:idmap v:ext="edit" data="1" /> </o:shapelayout></xml><![endif]--></head><body lang=SL link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span lang=EN-GB style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Hi,<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-GB style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I shall happily rewrite the code to remove those declarations. However, I'll need to check MSDN on a case-by-case basis to replace only API calls that are standard from Vista on.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-GB style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Do you still maintain 2.3 branch for Windows XP? You do know, that replacing GetProcAddr() with statically linked Vista+ API calls would break compatibility with Windows XP.<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span lang=EN-GB style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Best regards,<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Simon<o:p></o:p></span></p><p class=MsoNormal><span lang=EN-GB style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><div style='border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt'><div><div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm'><p class=MsoNormal><b><span lang=EN-US style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span lang=EN-US style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> Selva [mailto:selva.nair@gmail.com] <br><b>Sent:</b> Wednesday, October 11, 2017 3:21 PM<br><b>To:</b> Simon Rozman<br><b>Cc:</b> openvpn-devel@lists.sourceforge.net<br><b>Subject:</b> Re: [Openvpn-devel] [PATCH 05/13] Function prototypes are declared as "typedef <return type> (<calling convention> *type_name)(<arguments>)" in MSVC.<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal>Hi,<o:p></o:p></p><div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Thanks for these patches.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal>On Tue, Oct 10, 2017 at 7:11 PM, <<a href="mailto:simon@rozman.si" target="_blank">simon@rozman.si</a>> wrote:<o:p></o:p></p><p class=MsoNormal>From: Simon Rozman <<a href="mailto:simon@rozman.si" target="_blank">simon@rozman.si</a>><br><br>Note: NETIOAPI_API is defined as:<br>#define NETIOAPI_API NETIO_STATUS NETIOAPI_API_<br>#define NETIOAPI_API_ WINAPI<br><br>I am not sure whether interactive.c will compile using mingw with this patch. If not:<br>1. We can introduce some<br> #ifdef _MSC_VER<br> #else<br> #endif<br>2. Since NTDDI_VERSION=NTDDI_VISTA, most of the dynamic API loading by GetProcAddress() could be simplified to statically linked API calls, avoiding function prototype declarations altogether. I suggest we do this anyway to clean-up the code in the future.<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Yes, those declarations and run-time lookup were slated for removal but somehow never happened. Its a good idea to get rid of those -- no point in tweaking them to suit MSVC. <o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>IIRC, at least the versions of mingw that I use (gcc 4.9.3 or later) will build with those stripped but I can test again if you replace this patch with one removing those declarations.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Selva<o:p></o:p></p></div></div></div></div></div></div></div></body></html> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, Do you still maintain 2.3 branch for Windows XP? You do know, that > replacing GetProcAddr() with statically linked Vista+ API calls would break > compatibility with Windows XP. > The interactive service is only available in 2.4 and above and was never released for XP. Those dynamic lookups are a holdover from the original patch that implemented this feature and somehow never got removed. All the calls involved are supported in vista and above and removing all those GetProcAddr() calls in interactive.c should work -- indeed there was a patch submitted by someone for this which never got much traction mainly because it was for an older version of MSVC which did not support C99 and thus included other changes. Selva <div dir="ltr">Hi,<div><br></div><div><br></div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="SL" link="blue" vlink="purple"><div class="m_-7726908698724198423m_7605995917208064992WordSection1"><p class="MsoNormal"><span lang="EN-GB" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Do you still maintain 2.3 branch for Windows XP? You do know, that replacing GetProcAddr() with statically linked Vista+ API calls would break compatibility with Windows XP.</span></p></div></div></blockquote><div><br></div><div>The interactive service is only available in 2.4 and above and was never released for XP. Those dynamic lookups are a holdover from the original patch that implemented this feature and somehow never got removed. All the calls involved are supported in vista and above and removing all those GetProcAddr() calls in interactive.c should work -- indeed there was a patch submitted by someone for this which never got much traction mainly because it was for an older version of MSVC which did not support C99 and thus included other changes.</div><div><br></div><div>Selva</div></div></div></div> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Wed, Oct 11, 2017 at 01:57:03PM +0000, Simon Rozman wrote: > Do you still maintain 2.3 branch for Windows XP? You do know, that replacing > GetProcAddr() with statically linked Vista+ API calls would break > compatibility with Windows XP. These patches won't go into 2.3 - 2.3 is in strict maintenance mode, and the code base is sufficiently different (especially wrt to openvpn interactive service, which is not in the 2.3 code base and not needed on XP anyway) that it would need more effort for backporting. In other words: just ignore XP. gert
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index 7ff45b1..8d94197 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -550,7 +550,7 @@ InterfaceLuid(const char *iface_name, PNET_LUID luid) LPWSTR wide_name; int n; - typedef NETIO_STATUS WINAPI (*ConvertInterfaceAliasToLuidFn) (LPCWSTR, PNET_LUID); + typedef NETIO_STATUS (WINAPI *ConvertInterfaceAliasToLuidFn) (LPCWSTR, PNET_LUID); static ConvertInterfaceAliasToLuidFn ConvertInterfaceAliasToLuid = NULL; if (!ConvertInterfaceAliasToLuid) { @@ -585,7 +585,7 @@ CmpAddress(LPVOID item, LPVOID address) static DWORD DeleteAddress(PMIB_UNICASTIPADDRESS_ROW addr_row) { - typedef NETIOAPI_API (*DeleteUnicastIpAddressEntryFn) (const PMIB_UNICASTIPADDRESS_ROW); + typedef NETIO_STATUS (NETIOAPI_API_ *DeleteUnicastIpAddressEntryFn) (const PMIB_UNICASTIPADDRESS_ROW); static DeleteUnicastIpAddressEntryFn DeleteUnicastIpAddressEntry = NULL; if (!DeleteUnicastIpAddressEntry) @@ -613,8 +613,8 @@ HandleAddressMessage(address_message_t *msg, undo_lists_t *lists) PMIB_UNICASTIPADDRESS_ROW addr_row; BOOL add = msg->header.type == msg_add_address; - typedef NETIOAPI_API (*CreateUnicastIpAddressEntryFn) (const PMIB_UNICASTIPADDRESS_ROW); - typedef NETIOAPI_API (*InitializeUnicastIpAddressEntryFn) (PMIB_UNICASTIPADDRESS_ROW); + typedef NETIO_STATUS (NETIOAPI_API_ *CreateUnicastIpAddressEntryFn) (const PMIB_UNICASTIPADDRESS_ROW); + typedef NETIO_STATUS (NETIOAPI_API_ *InitializeUnicastIpAddressEntryFn) (PMIB_UNICASTIPADDRESS_ROW); static CreateUnicastIpAddressEntryFn CreateUnicastIpAddressEntry = NULL; static InitializeUnicastIpAddressEntryFn InitializeUnicastIpAddressEntry = NULL; @@ -707,7 +707,7 @@ CmpRoute(LPVOID item, LPVOID route) static DWORD DeleteRoute(PMIB_IPFORWARD_ROW2 fwd_row) { - typedef NETIOAPI_API (*DeleteIpForwardEntry2Fn) (PMIB_IPFORWARD_ROW2); + typedef NETIO_STATUS (NETIOAPI_API_ *DeleteIpForwardEntry2Fn) (PMIB_IPFORWARD_ROW2); static DeleteIpForwardEntry2Fn DeleteIpForwardEntry2 = NULL; if (!DeleteIpForwardEntry2) @@ -735,7 +735,7 @@ HandleRouteMessage(route_message_t *msg, undo_lists_t *lists) PMIB_IPFORWARD_ROW2 fwd_row; BOOL add = msg->header.type == msg_add_route; - typedef NETIOAPI_API (*CreateIpForwardEntry2Fn) (PMIB_IPFORWARD_ROW2); + typedef NETIO_STATUS (NETIOAPI_API_ *CreateIpForwardEntry2Fn) (PMIB_IPFORWARD_ROW2); static CreateIpForwardEntry2Fn CreateIpForwardEntry2 = NULL; if (!CreateIpForwardEntry2) @@ -821,7 +821,7 @@ out: static DWORD HandleFlushNeighborsMessage(flush_neighbors_message_t *msg) { - typedef NETIOAPI_API (*FlushIpNetTable2Fn) (ADDRESS_FAMILY, NET_IFINDEX); + typedef NETIO_STATUS (NETIOAPI_API_ *FlushIpNetTable2Fn) (ADDRESS_FAMILY, NET_IFINDEX); static FlushIpNetTable2Fn flush_fn = NULL; if (msg->family == AF_INET)
From: Simon Rozman <simon@rozman.si> Note: NETIOAPI_API is defined as: #define NETIOAPI_API NETIO_STATUS NETIOAPI_API_ #define NETIOAPI_API_ WINAPI I am not sure whether interactive.c will compile using mingw with this patch. If not: 1. We can introduce some #ifdef _MSC_VER #else #endif 2. Since NTDDI_VERSION=NTDDI_VISTA, most of the dynamic API loading by GetProcAddress() could be simplified to statically linked API calls, avoiding function prototype declarations altogether. I suggest we do this anyway to clean-up the code in the future. --- src/openvpnserv/interactive.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)