<div>Hi Sebastian,</div><div>š</div><div>Thanks for you feedback here and in IRC, you could decline the patches. I will re-master them soon</div><div>š</div><div>--de</div><div>š</div><div>07.01.2016, 03:44, "Sebastian Lackner" <sebastian@fds-team.de>:</div><blockquote type="cite"><p>Thanks for your contribution. I didn't do a full review yet, but there are a<br />couple of things I would like to point out. Before you resend, please also<br />wait if other people have additional comments.<br /><br />Sorry in advance, the Wine project can sometimes be a bit picky with patch<br />acceptance. ;) Please note that especially for people new to the Wine project,<br />it is highly recommended to send your patches splitted in small pieces.<br />In this case for example, you could send one patch per function, with<br />corresponding test patch inbetween, to make reviewing as easy as possible.<br /><br />On 07.01.2016 00:25, Donat Enikeev wrote:</p><blockquote>šUsing inet_?to? if available, otherwise - behave like a stub.<br /><br />šTested on Windows8@VM and Ubutnu 15.04.<br /><br />šSigned-off-by: Donat Enikeev <<a href="mailto:donat@enikeev.net">donat@enikeev.net</a>><br /><br />š---<br />ššdlls/ntdll/ntdll.spec | 6 +--<br />ššdlls/ntdll/rtl.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++<br />ššinclude/in6addr.h | 4 ++<br />šš3 files changed, 136 insertions(+), 3 deletions(-)<br /><br />šdiff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec<br />šindex 8cabf95..d7c23fc 100644<br />š--- a/dlls/ntdll/ntdll.spec<br />š+++ b/dlls/ntdll/ntdll.spec<br />š@@ -711,14 +711,14 @@<br />šš@ stdcall RtlIpv4AddressToStringExA(ptr long ptr ptr)<br />šš@ stdcall RtlIpv4AddressToStringExW(ptr long ptr ptr)<br />šš@ stdcall RtlIpv4AddressToStringW(ptr ptr)<br />š-# @ stub RtlIpv4StringToAddressA<br />š+@ stdcall RtlIpv4StringToAddressA(ptr long ptr ptr)</blockquote><p><br />For strings (like the first argument), str should be used.<br /><br /></p><blockquote>šš# @ stub RtlIpv4StringToAddressExA<br />šš@ stdcall RtlIpv4StringToAddressExW(ptr ptr wstr ptr)<br />šš# @ stub RtlIpv4StringToAddressW<br />š-# @ stub RtlIpv6AddressToStringA<br />š+@ stdcall RtlIpv6AddressToStringA(ptr ptr)<br />šš# @ stub RtlIpv6AddressToStringExA<br />šš# @ stub RtlIpv6AddressToStringExW<br />š-# @ stub RtlIpv6AddressToStringW<br />š+@ stdcall RtlIpv6AddressToStringW(ptr ptr)</blockquote><p><br />Usually *A functions are implemented on top of the corresponding<br />*W function. This means you have to implement RtlIpv4StringToAddressW<br />first, and then RtlIpv4StringToAddressA on top of it.<br /><br /></p><blockquote>šš# @ stub RtlIpv6StringToAddressA<br />šš# @ stub RtlIpv6StringToAddressExA<br />šš# @ stub RtlIpv6StringToAddressExW<br />šdiff --git a/dlls/ntdll/rtl.c b/dlls/ntdll/rtl.c<br />šindex 74ad35a..5c82d<span>43 100644</span><br />š--- a/dlls/ntdll/rtl.c<br />š+++ b/dlls/ntdll/rtl.c<br />š@@ -33,6 +33,9 @@<br />šš#ifdef HAVE_NETINET_IN_H<br />šš#include <netinet/in.h><br />šš#endif<br />š+#ifdef HAVE_ARPA_INET_H<br />š+#include <arpa/inet.h><br />š+#endif<br />šš#include "ntstatus.h"<br />šš#define NONAMELESSUNION<br />šš#define NONAMELESSSTRUCT<br />š@@ -45,6 +48,7 @@<br />šš#include "wine/unicode.h"<br />šš#include "ntdll_misc.h"<br />šš#include "inaddr.h"<br />š+#include "in6addr.h"<br />šš#include "ddk/ntddk.h"<br /><br />ššWINE_DEFAULT_DEBUG_CHANNEL(ntdll);<br />š@@ -891,6 +895,50 @@ NTSTATUS WINAPI RtlIpv4StringToAddressExW(PULONG IP, PULONG Port,<br />šš}<br /><br />šš/***********************************************************************<br />š+ * RtlIpv4StringToAddressA [NTDLL.@]<br />š+ *<br />š+ * Convert the given ipv4 address to binary representation (network byte order)<br />š+ *<br />š+ * PARAMS<br />š+ * s [I] NULL terminated string with address to convert<br />š+ * strict [I] TRUE: restricts to usual 4-part ip-address with decimal parts; FALSE: other forms allowed<br />š+ * terminator [O] Pointer to the terminated charatcer of s<br />š+ * in_addr [IO] PTR to store converted ip address<br />š+ *<br />š+ * RETURNS<br />š+ * Success: STATUS_SUCCESS<br />š+ * Failure: STATUS_INVALID_PARAMETER<br />š+ *<br />š+ */<br />š+NTSTATUS WINAPI RtlIpv4StringToAddressA(LPCSTR s, BOOLEAN strict, LPCSTR *terminator, IN_ADDR *in_addr)</blockquote><p><br />As mentioned above, usually *A functions forward to the corresponding *W function.<br />It might be preferred to avoid using inet_pton(), and use direct string operations instead.<br /><br /></p><blockquote>š+{<br />š+ if (!s || !in_addr)<br />š+ return STATUS_INVALID_PARAMETER;<br />š+<br />š+ if (strict == FALSE)<br />š+ FIXME("Support of non-decimal IP addresses is not implemented. semi-stub (%s)\n", s);<br />š+ /* ^ support of the option above could be easily implemented with `inet_atop`, not available by the moment in wine */<br />š+<br />š+ #ifdef HAVE_INET_PTON<br />š+<br />š+ TRACE("('%s' @ %s, %d, %p, %p, %llu) \n", s, s, strict, terminator, in_addr, (long long unsigned)in_addr->S_un.S_addr);</blockquote><p><br />Use debugstr_a / debugstr_w() for tracing untrusted values. Why do you print s twice?<br /><br /></p><blockquote>š+<br />š+ if (!inet_pton(AF_INET, s, &in_addr->S_un.S_addr))<br />š+ {<br />š+ TRACE("Conversion of ip address '%s' failed\n", s);<br />š+ return STATUS_INVALID_PARAMETER;<br />š+ }<br />š+<br />š+ *terminator = s+(int)strlen(s);<br />š+<br />š+ return STATUS_SUCCESS;<br />š+ #endif<br />š+<br />š+ FIXME( "No inet_pton() available on the platform - stub\n" );<br />š+ return STATUS_INVALID_PARAMETER;<br />š+}<br />š+<br />š+/***********************************************************************<br />ššš* RtlIpv4AddressToStringExW [NTDLL.@]<br />ššš*<br />ššš* Convert the given ipv4 address and optional the port to a string<br />š@@ -1004,6 +1052,87 @@ CHAR * WINAPI RtlIpv4AddressToStringA(const IN_ADDR *pin, LPSTR buffer)<br />šš}<br /><br />šš/***********************************************************************<br />š+ * RtlIpv6AddressToStringA [NTDLL.@]<br />š+ *<br />š+ * Converts the given ipv6 address to a string<br />š+ *<br />š+ * PARAMS<br />š+ * pin [I] PTR to the ip address to convert (network byte order)<br />š+ * buffer [O] destination buffer for the result (at least 46 characters)<br />š+ *<br />š+ * RETURNS<br />š+ * PTR to the 0 character at the end of the converted string<br />š+ *<br />š+ */<br />š+CHAR * WINAPI RtlIpv6AddressToStringA(const IN6_ADDR *pin, LPSTR buffer)<br />š+{</blockquote><p><br />Please add TRACES or FIXMEs (for partial implementations) at the top of each function.<br /><br /></p><blockquote>š+ if (!pin || !buffer)<br />š+ {<br />š+ WARN("Incorrect parameters passed: (%p, %p)", pin, buffer);</blockquote><p><br />Always add a linebreak at the end of messages.<br /><br /></p><blockquote>š+ return NULL;<br />š+ }<br />š+<br />š+ #ifdef HAVE_INET_NTOP<br />š+<br />š+ if (!inet_ntop(AF_INET6, &(pin->u), buffer, INET6_ADDRSTRLEN))<br />š+ ERR("Conversion of Ipv6Address failed\n");<br />š+<br />š+ TRACE("(%p, %p) Ipv6 Address %s\n", pin, buffer, buffer);<br />š+<br />š+ return buffer+strlen(buffer);<br />š+<br />š+ #else<br />š+<br />š+ FIXME( "No inet_ntop() available - stub\n" );<br />š+ return buffer;<br />š+<br />š+ #endif<br />š+}<br />š+<br />š+/***********************************************************************<br />š+ * RtlIpv6AddressToStringW [NTDLL.@]<br />š+ *<br />š+ * Converts the given ipv6 address to a string<br />š+ *<br />š+ * PARAMS<br />š+ * pin [I] PTR to the ip address to convert (network byte order)<br />š+ * buffer [O] destination buffer for the result (at least 46 characters)<br />š+ *<br />š+ * RETURNS<br />š+ * PTR to the 0 character at the end of the converted string<br />š+ *<br />š+ */<br />š+WCHAR * WINAPI RtlIpv6AddressToStringW(const IN6_ADDR *pin, LPWSTR buffer)<br />š+{<br />š+ CHAR tmp_buffer[INET6_ADDRSTRLEN];</blockquote><p><br />Here a TRACE/FIXME is also missing.<br /><br /></p><blockquote>š+<br />š+ if (!pin || !buffer)<br />š+ {<br />š+ WARN("Incorrect parameters passed: (%p, %p)", pin, buffer);<br />š+ return NULL;<br />š+ }<br />š+<br />š+ if (RtlIpv6AddressToStringA(pin, (LPSTR)&tmp_buffer))</blockquote><p><br />There are many places in your patch with unncessary casts. Those should be avoided.<br /><br /></p><blockquote>š+ {<br />š+ int len = strlen(tmp_buffer);<br />š+<br />š+ RtlZeroMemory(buffer, len*sizeof(WCHAR));</blockquote><p><br />This doesn't work when your string contains multibyte characters.<br /><br /></p><blockquote>š+<br />š+ if (len > INET6_ADDRSTRLEN)<br />š+ WARN("Potential buffer overflow with %d length of %d maximum supposed", len, INET6_ADDRSTRLEN);</blockquote><p><br />Unless the same limitation exists on Windows, such code should be avoided.<br />Also please note that INET6_ADDRSTRLEN is defined different on Linux/Windows.<br /><br /></p><blockquote>š+<br />š+ TRACE("Got Ipv6Address '%s' of length %d from RtlIpv6AddressToStringA\n", (char *)&tmp_buffer, len);<br />š+<br />š+ ntdll_umbstowcs(0, (const char *)&tmp_buffer, len, buffer, len);<br />š+<br />š+ return buffer+len;<br />š+ }<br />š+<br />š+ FIXME("(%p, %p): semi-stub\n", pin, buffer);<br />š+ return buffer;<br />š+}<br />š+<br />š+/***********************************************************************<br />ššš* get_pointer_obfuscator (internal)<br />ššš*/<br />ššstatic DWORD_PTR get_pointer_obfuscator( void )<br />šdiff --git a/include/in6addr.h b/include/in6addr.h<br />šindex a745bd2..<span>7600407 100644</span><br />š--- a/include/in6addr.h<br />š+++ b/include/in6addr.h<br />š@@ -32,6 +32,10 @@ typedef struct WS(in6_addr) {<br />šššššš} u;<br />šš} IN6_ADDR, *PIN6_ADDR, *LPIN6_ADDR;<br /><br />š+#ifndef INET6_ADDRSTRLEN<br />š+#define INET6_ADDRSTRLEN 65<br />š+#endif<br />š+<br />šš#define in_addr6 WS(in6_addr)<br /><br />šš#ifdef USE_WS_PREFIX<br /><br /></blockquote><p>š</p></blockquote>