Hi Sebastian,
š
Thanks for you feedback here and in IRC, you could decline the patches. I will re-master them soon
š
--de
š
07.01.2016, 03:44, "Sebastian Lackner" <sebastian@fds-team.de>:

Thanks for your contribution. I didn't do a full review yet, but there are a
couple of things I would like to point out. Before you resend, please also
wait if other people have additional comments.

Sorry in advance, the Wine project can sometimes be a bit picky with patch
acceptance. ;) Please note that especially for people new to the Wine project,
it is highly recommended to send your patches splitted in small pieces.
In this case for example, you could send one patch per function, with
corresponding test patch inbetween, to make reviewing as easy as possible.

On 07.01.2016 00:25, Donat Enikeev wrote:

šUsing inet_?to? if available, otherwise - behave like a stub.

šTested on Windows8@VM and Ubutnu 15.04.

šSigned-off-by: Donat Enikeev <donat@enikeev.net>

š---
ššdlls/ntdll/ntdll.spec | 6 +--
ššdlls/ntdll/rtl.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++
ššinclude/in6addr.h | 4 ++
šš3 files changed, 136 insertions(+), 3 deletions(-)

šdiff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec
šindex 8cabf95..d7c23fc 100644
š--- a/dlls/ntdll/ntdll.spec
š+++ b/dlls/ntdll/ntdll.spec
š@@ -711,14 +711,14 @@
šš@ stdcall RtlIpv4AddressToStringExA(ptr long ptr ptr)
šš@ stdcall RtlIpv4AddressToStringExW(ptr long ptr ptr)
šš@ stdcall RtlIpv4AddressToStringW(ptr ptr)
š-# @ stub RtlIpv4StringToAddressA
š+@ stdcall RtlIpv4StringToAddressA(ptr long ptr ptr)


For strings (like the first argument), str should be used.

šš# @ stub RtlIpv4StringToAddressExA
šš@ stdcall RtlIpv4StringToAddressExW(ptr ptr wstr ptr)
šš# @ stub RtlIpv4StringToAddressW
š-# @ stub RtlIpv6AddressToStringA
š+@ stdcall RtlIpv6AddressToStringA(ptr ptr)
šš# @ stub RtlIpv6AddressToStringExA
šš# @ stub RtlIpv6AddressToStringExW
š-# @ stub RtlIpv6AddressToStringW
š+@ stdcall RtlIpv6AddressToStringW(ptr ptr)


Usually *A functions are implemented on top of the corresponding
*W function. This means you have to implement RtlIpv4StringToAddressW
first, and then RtlIpv4StringToAddressA on top of it.

šš# @ stub RtlIpv6StringToAddressA
šš# @ stub RtlIpv6StringToAddressExA
šš# @ stub RtlIpv6StringToAddressExW
šdiff --git a/dlls/ntdll/rtl.c b/dlls/ntdll/rtl.c
šindex 74ad35a..5c82d43 100644
š--- a/dlls/ntdll/rtl.c
š+++ b/dlls/ntdll/rtl.c
š@@ -33,6 +33,9 @@
šš#ifdef HAVE_NETINET_IN_H
šš#include <netinet/in.h>
šš#endif
š+#ifdef HAVE_ARPA_INET_H
š+#include <arpa/inet.h>
š+#endif
šš#include "ntstatus.h"
šš#define NONAMELESSUNION
šš#define NONAMELESSSTRUCT
š@@ -45,6 +48,7 @@
šš#include "wine/unicode.h"
šš#include "ntdll_misc.h"
šš#include "inaddr.h"
š+#include "in6addr.h"
šš#include "ddk/ntddk.h"

ššWINE_DEFAULT_DEBUG_CHANNEL(ntdll);
š@@ -891,6 +895,50 @@ NTSTATUS WINAPI RtlIpv4StringToAddressExW(PULONG IP, PULONG Port,
šš}

šš/***********************************************************************
š+ * RtlIpv4StringToAddressA [NTDLL.@]
š+ *
š+ * Convert the given ipv4 address to binary representation (network byte order)
š+ *
š+ * PARAMS
š+ * s [I] NULL terminated string with address to convert
š+ * strict [I] TRUE: restricts to usual 4-part ip-address with decimal parts; FALSE: other forms allowed
š+ * terminator [O] Pointer to the terminated charatcer of s
š+ * in_addr [IO] PTR to store converted ip address
š+ *
š+ * RETURNS
š+ * Success: STATUS_SUCCESS
š+ * Failure: STATUS_INVALID_PARAMETER
š+ *
š+ */
š+NTSTATUS WINAPI RtlIpv4StringToAddressA(LPCSTR s, BOOLEAN strict, LPCSTR *terminator, IN_ADDR *in_addr)


As mentioned above, usually *A functions forward to the corresponding *W function.
It might be preferred to avoid using inet_pton(), and use direct string operations instead.

š+{
š+ if (!s || !in_addr)
š+ return STATUS_INVALID_PARAMETER;
š+
š+ if (strict == FALSE)
š+ FIXME("Support of non-decimal IP addresses is not implemented. semi-stub (%s)\n", s);
š+ /* ^ support of the option above could be easily implemented with `inet_atop`, not available by the moment in wine */
š+
š+ #ifdef HAVE_INET_PTON
š+
š+ TRACE("('%s' @ %s, %d, %p, %p, %llu) \n", s, s, strict, terminator, in_addr, (long long unsigned)in_addr->S_un.S_addr);


Use debugstr_a / debugstr_w() for tracing untrusted values. Why do you print s twice?

š+
š+ if (!inet_pton(AF_INET, s, &in_addr->S_un.S_addr))
š+ {
š+ TRACE("Conversion of ip address '%s' failed\n", s);
š+ return STATUS_INVALID_PARAMETER;
š+ }
š+
š+ *terminator = s+(int)strlen(s);
š+
š+ return STATUS_SUCCESS;
š+ #endif
š+
š+ FIXME( "No inet_pton() available on the platform - stub\n" );
š+ return STATUS_INVALID_PARAMETER;
š+}
š+
š+/***********************************************************************
ššš* RtlIpv4AddressToStringExW [NTDLL.@]
ššš*
ššš* Convert the given ipv4 address and optional the port to a string
š@@ -1004,6 +1052,87 @@ CHAR * WINAPI RtlIpv4AddressToStringA(const IN_ADDR *pin, LPSTR buffer)
šš}

šš/***********************************************************************
š+ * RtlIpv6AddressToStringA [NTDLL.@]
š+ *
š+ * Converts the given ipv6 address to a string
š+ *
š+ * PARAMS
š+ * pin [I] PTR to the ip address to convert (network byte order)
š+ * buffer [O] destination buffer for the result (at least 46 characters)
š+ *
š+ * RETURNS
š+ * PTR to the 0 character at the end of the converted string
š+ *
š+ */
š+CHAR * WINAPI RtlIpv6AddressToStringA(const IN6_ADDR *pin, LPSTR buffer)
š+{


Please add TRACES or FIXMEs (for partial implementations) at the top of each function.

š+ if (!pin || !buffer)
š+ {
š+ WARN("Incorrect parameters passed: (%p, %p)", pin, buffer);


Always add a linebreak at the end of messages.

š+ return NULL;
š+ }
š+
š+ #ifdef HAVE_INET_NTOP
š+
š+ if (!inet_ntop(AF_INET6, &(pin->u), buffer, INET6_ADDRSTRLEN))
š+ ERR("Conversion of Ipv6Address failed\n");
š+
š+ TRACE("(%p, %p) Ipv6 Address %s\n", pin, buffer, buffer);
š+
š+ return buffer+strlen(buffer);
š+
š+ #else
š+
š+ FIXME( "No inet_ntop() available - stub\n" );
š+ return buffer;
š+
š+ #endif
š+}
š+
š+/***********************************************************************
š+ * RtlIpv6AddressToStringW [NTDLL.@]
š+ *
š+ * Converts the given ipv6 address to a string
š+ *
š+ * PARAMS
š+ * pin [I] PTR to the ip address to convert (network byte order)
š+ * buffer [O] destination buffer for the result (at least 46 characters)
š+ *
š+ * RETURNS
š+ * PTR to the 0 character at the end of the converted string
š+ *
š+ */
š+WCHAR * WINAPI RtlIpv6AddressToStringW(const IN6_ADDR *pin, LPWSTR buffer)
š+{
š+ CHAR tmp_buffer[INET6_ADDRSTRLEN];


Here a TRACE/FIXME is also missing.

š+
š+ if (!pin || !buffer)
š+ {
š+ WARN("Incorrect parameters passed: (%p, %p)", pin, buffer);
š+ return NULL;
š+ }
š+
š+ if (RtlIpv6AddressToStringA(pin, (LPSTR)&tmp_buffer))


There are many places in your patch with unncessary casts. Those should be avoided.

š+ {
š+ int len = strlen(tmp_buffer);
š+
š+ RtlZeroMemory(buffer, len*sizeof(WCHAR));


This doesn't work when your string contains multibyte characters.

š+
š+ if (len > INET6_ADDRSTRLEN)
š+ WARN("Potential buffer overflow with %d length of %d maximum supposed", len, INET6_ADDRSTRLEN);


Unless the same limitation exists on Windows, such code should be avoided.
Also please note that INET6_ADDRSTRLEN is defined different on Linux/Windows.

š+
š+ TRACE("Got Ipv6Address '%s' of length %d from RtlIpv6AddressToStringA\n", (char *)&tmp_buffer, len);
š+
š+ ntdll_umbstowcs(0, (const char *)&tmp_buffer, len, buffer, len);
š+
š+ return buffer+len;
š+ }
š+
š+ FIXME("(%p, %p): semi-stub\n", pin, buffer);
š+ return buffer;
š+}
š+
š+/***********************************************************************
ššš* get_pointer_obfuscator (internal)
ššš*/
ššstatic DWORD_PTR get_pointer_obfuscator( void )
šdiff --git a/include/in6addr.h b/include/in6addr.h
šindex a745bd2..7600407 100644
š--- a/include/in6addr.h
š+++ b/include/in6addr.h
š@@ -32,6 +32,10 @@ typedef struct WS(in6_addr) {
šššššš} u;
šš} IN6_ADDR, *PIN6_ADDR, *LPIN6_ADDR;

š+#ifndef INET6_ADDRSTRLEN
š+#define INET6_ADDRSTRLEN 65
š+#endif
š+
šš#define in_addr6 WS(in6_addr)

šš#ifdef USE_WS_PREFIX

š