[PATCH 1/2] ntdll: RtlIpv4|6 implementations
Sebastian Lackner
sebastian at fds-team.de
Wed Jan 6 18:44:51 CST 2016
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 at VM and Ubutnu 15.04.
>
> Signed-off-by: Donat Enikeev <donat at 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
>
More information about the wine-devel
mailing list