[PATCH] Add missing RtlQueryEnvironmentVariable to ntdll
Zebediah Figura
z.figura12 at gmail.com
Sun May 3 09:59:11 CDT 2020
Hello Alon, thanks for the patch! I have a few comments inline:
On 5/3/20 2:11 AM, Alon Barzilai wrote:
> ---
> dlls/ntdll/env.c | 44 ++++++++++++++++++++++++++++++++++++++++++
> dlls/ntdll/ntdll.spec | 1 +
> dlls/ntdll/tests/env.c | 44 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 89 insertions(+)
>
> diff --git a/dlls/ntdll/env.c b/dlls/ntdll/env.c
> index 71ae48681d..61c4d56baa 100644
> --- a/dlls/ntdll/env.c
> +++ b/dlls/ntdll/env.c
> @@ -38,6 +38,7 @@
> #define WIN32_NO_STATUS
> #include "windef.h"
> #include "winternl.h"
> +#include "wine/library.h"
> #include "wine/debug.h"
> #include "ntdll_misc.h"
> #include "winnt.h"
You shouldn't need this, and you don't seem to use it either.
> @@ -990,6 +991,49 @@ NTSTATUS WINAPI RtlQueryEnvironmentVariable_U(PWSTR env,
> return nts;
> }
>
> +
> +/******************************************************************
> + * RtlQueryEnvironmentVariable [NTDLL.@]
> + *
> + */
> +NTSTATUS WINAPI RtlQueryEnvironmentVariable(PWSTR env,
> + PWSTR name,
> + size_t name_length,
> + PWSTR value,
> + size_t value_length,
> + size_t* return_length)
I think those size_t should be (capital) SIZE_T.
I think putting each parameter on a separate line is quite unnecessary.
We also tend to avoid P*/LP* typedefs in new code.
> +{
> + UNICODE_STRING name_u;
> + UNICODE_STRING value_u;
> + NTSTATUS nts;
> +
> + name_u.Length = name_length * sizeof(WCHAR);
> + name_u.MaximumLength = name_u.Length;
> + name_u.Buffer = name;
> +
> + value_u.Length = 0;
> + value_u.MaximumLength = value_length * sizeof(WCHAR);
> + value_u.Buffer = value;
> +
> + nts = RtlQueryEnvironmentVariable_U(env, &name_u, &value_u);
> + switch (nts)
> + {
> + case STATUS_SUCCESS:
> + *return_length = value_u.Length / sizeof(WCHAR);
> + break;
> +
> + case STATUS_BUFFER_TOO_SMALL:
> + *return_length = (value_u.Length / sizeof(WCHAR)) + 1;
> + break;
> +
> + case STATUS_VARIABLE_NOT_FOUND:
> + default:
> + *return_length = 0;
> + break;
> + }
I'm not sure this is enough cases to really merit a switch statement. If
you do, however, I'd get rid of the redundant STATUS_VARIABLE_NOT_FOUND
case, and indent code after a case label.
> + return nts;
> +}
> +
> /******************************************************************
> * RtlSetCurrentEnvironment [NTDLL.@]
> *
> diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec
> index 4f5fa8c21d..cbc43259bd 100644
> --- a/dlls/ntdll/ntdll.spec
> +++ b/dlls/ntdll/ntdll.spec
> @@ -860,6 +860,7 @@
> @ stdcall RtlQueryDepthSList(ptr)
> @ stdcall RtlQueryDynamicTimeZoneInformation(ptr)
> @ stdcall RtlQueryEnvironmentVariable_U(ptr ptr ptr)
> +@ stdcall RtlQueryEnvironmentVariable(ptr ptr long ptr long ptr)
> @ stdcall RtlQueryHeapInformation(long long ptr long ptr)
> @ stdcall RtlQueryInformationAcl(ptr ptr long long)
> @ stdcall RtlQueryInformationActivationContext(long long ptr long ptr long ptr)
> diff --git a/dlls/ntdll/tests/env.c b/dlls/ntdll/tests/env.c
> index 48c9ed809e..bb081e20ed 100644
> --- a/dlls/ntdll/tests/env.c
> +++ b/dlls/ntdll/tests/env.c
> @@ -27,6 +27,7 @@ static NTSTATUS (WINAPI *pRtlMultiByteToUnicodeN)( LPWSTR dst, DWORD dstlen, LPD
> static NTSTATUS (WINAPI *pRtlCreateEnvironment)(BOOLEAN, PWSTR*);
> static NTSTATUS (WINAPI *pRtlDestroyEnvironment)(PWSTR);
> static NTSTATUS (WINAPI *pRtlQueryEnvironmentVariable_U)(PWSTR, PUNICODE_STRING, PUNICODE_STRING);
> +static NTSTATUS (WINAPI* pRtlQueryEnvironmentVariable)(PWSTR, PWSTR, size_t, PWSTR, size_t, size_t*);
> static void (WINAPI *pRtlSetCurrentEnvironment)(PWSTR, PWSTR*);
> static NTSTATUS (WINAPI *pRtlSetEnvironmentVariable)(PWSTR*, PUNICODE_STRING, PUNICODE_STRING);
> static NTSTATUS (WINAPI *pRtlExpandEnvironmentStrings_U)(LPWSTR, PUNICODE_STRING, PUNICODE_STRING, PULONG);
> @@ -95,6 +96,9 @@ static void testQuery(void)
> UNICODE_STRING name;
> UNICODE_STRING value;
> NTSTATUS nts;
> + size_t name_length;
> + size_t value_length;
> + size_t return_length;
> unsigned int i;
>
> for (i = 0; tests[i].var; i++)
> @@ -130,6 +134,45 @@ static void testQuery(void)
> break;
> }
> }
> +
> + //test RtlQueryEnvironmentVariable
We don't use C++ comments, but I think this comment is quite redundant
in the first place.
> + if (pRtlQueryEnvironmentVariable)
> + {
> + for (i = 0; tests[i].var; i++)
> + {
> + const struct test* test = &tests[i];
> + name_length = strlen(test->var);
> + value_length = test->len;
> + value.Buffer = bv;
> + bv[test->len] = '@';
> +
> + pRtlMultiByteToUnicodeN(bn, sizeof(bn), NULL, test->var, strlen(test->var) + 1);
> + nts = pRtlQueryEnvironmentVariable(small_env, bn, name_length, bv, value_length, &return_length);
> + ok(nts == test->status || (test->alt && nts == test->alt),
> + "[%d]: Wrong status for '%s', expecting %x got %x\n",
> + i, test->var, test->status, nts);
> + if (nts == test->status) switch (nts)
> + {
> + case STATUS_SUCCESS:
> + pRtlMultiByteToUnicodeN(bn, sizeof(bn), NULL, test->val, strlen(test->val) + 1);
> + ok(return_length == strlen(test->val), "Wrong length %d for %s\n",
> + return_length, test->var);
> + ok((return_length == strlen(test->val) && memcmp(bv, bn, return_length) == 0) ||
> + lstrcmpW(bv, bn) == 0,
> + "Wrong result for %s/%d\n", test->var, test->len);
> + ok(bv[test->len] == '@', "Writing too far away in the buffer for %s/%d\n", test->var, test->len);
> + break;
> + case STATUS_BUFFER_TOO_SMALL:
> + ok(return_length == (strlen(test->val) + 1),
> + "Wrong returned length %d (too small buffer) for %s\n", return_length, test->var);
> + break;
> + }
> + }
> + }
> + else
> + {
> + win_skip("RtlQueryEnvironmentVariable not available, skipping tests\n");
> + }
> }
>
> static void testSetHelper(LPWSTR* env, const char* var, const char* val, NTSTATUS ret, NTSTATUS alt)
> @@ -525,6 +568,7 @@ START_TEST(env)
> pRtlCreateEnvironment = (void*)GetProcAddress(mod, "RtlCreateEnvironment");
> pRtlDestroyEnvironment = (void*)GetProcAddress(mod, "RtlDestroyEnvironment");
> pRtlQueryEnvironmentVariable_U = (void*)GetProcAddress(mod, "RtlQueryEnvironmentVariable_U");
> + pRtlQueryEnvironmentVariable = (void*)GetProcAddress(mod, "RtlQueryEnvironmentVariable");
> pRtlSetCurrentEnvironment = (void*)GetProcAddress(mod, "RtlSetCurrentEnvironment");
> pRtlSetEnvironmentVariable = (void*)GetProcAddress(mod, "RtlSetEnvironmentVariable");
> pRtlExpandEnvironmentStrings_U = (void*)GetProcAddress(mod, "RtlExpandEnvironmentStrings_U");
>
More information about the wine-devel
mailing list