Re: [ros-dev] [ros-diffs] [hbelusca] 57504: [NTOSKRNL] Rearrange the NtQuerySystemEnvironmentValue code to have successive logical checks.
6 Oct
2012
6 Oct
'12
8:20 p.m.
This changes the logic. I'm not sure how exact the previous version was,
but these changes should be tested.
Am 06.10.2012 21:50, schrieb hbelusca(a)svn.reactos.org:
Author: hbelusca
Date: Sat Oct 6 19:50:17 2012
New Revision: 57504
URL: http://svn.reactos.org/svn/reactos?rev=57504&view=rev
Log:
[NTOSKRNL]
Rearrange the NtQuerySystemEnvironmentValue code to have successive logical checks.
Modified:
trunk/reactos/ntoskrnl/ex/sysinfo.c
Modified: trunk/reactos/ntoskrnl/ex/sysinfo.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ex/sysinfo.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/ex/sysinfo.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ex/sysinfo.c [iso-8859-1] Sat Oct 6 19:50:17 2012
@@ -235,44 +235,40 @@
_SEH2_YIELD(return _SEH2_GetExceptionCode());
}
_SEH2_END;
-
- }
-
- /* Allocate a buffer for the value */
- AnsiValueBuffer = ExAllocatePoolWithTag(NonPagedPool, ValueBufferLength,
'pmeT');
- if (AnsiValueBuffer == NULL)
- {
- return STATUS_INSUFFICIENT_RESOURCES;
- }
-
- /*
- * Copy the name to kernel space if necessary and convert it to ANSI.
- */
+ }
+
+ /* According to NTInternals the SeSystemEnvironmentName privilege is required! */
+ if (!SeSinglePrivilegeCheck(SeSystemEnvironmentPrivilege, PreviousMode))
+ {
+ DPRINT1("NtQuerySystemEnvironmentValue: Caller requires the
SeSystemEnvironmentPrivilege privilege!\n");
+ return STATUS_PRIVILEGE_NOT_HELD;
+ }
+
+ /* Copy the name to kernel space if necessary */
Status = ProbeAndCaptureUnicodeString(&WName, PreviousMode, VariableName);
if (!NT_SUCCESS(Status))
{
return Status;
}
- /*
- * according to ntinternals the SeSystemEnvironmentName privilege is required!
- */
- if (!SeSinglePrivilegeCheck(SeSystemEnvironmentPrivilege, PreviousMode))
- {
- ReleaseCapturedUnicodeString(&WName, PreviousMode);
- DPRINT1("NtQuerySystemEnvironmentValue: Caller requires the
SeSystemEnvironmentPrivilege privilege!\n");
- return STATUS_PRIVILEGE_NOT_HELD;
- }
-
- /* Convert the value name to ansi and release the captured unicode string */
+ /* Convert the name to ANSI and release the captured UNICODE string */
Status = RtlUnicodeStringToAnsiString(&AName, &WName, TRUE);
ReleaseCapturedUnicodeString(&WName, PreviousMode);
if (!NT_SUCCESS(Status)) return Status;
- /* Get the environment variable */
+ /* Allocate a buffer for the ANSI environment variable */
+ AnsiValueBuffer = ExAllocatePoolWithTag(NonPagedPool, ValueBufferLength,
'pmeT');
+ if (AnsiValueBuffer == NULL)
+ {
+ RtlFreeAnsiString(&AName);
+ return STATUS_INSUFFICIENT_RESOURCES;
+ }
+
+ /* Get the environment variable and free the ANSI name */
Result = HalGetEnvironmentVariable(AName.Buffer,
(USHORT)ValueBufferLength,
AnsiValueBuffer);
+ RtlFreeAnsiString(&AName);
/* Check if we had success */
if (Result == ESUCCESS)
@@ -280,13 +276,13 @@
/* Copy the result back to the caller. */
_SEH2_TRY
{
- /* Initialize ansi string from the result */
+ /* Initialize ANSI string from the result */
RtlInitAnsiString(&AValue, AnsiValueBuffer);
- /* Initialize a unicode string from the callers buffer */
+ /* Initialize a UNICODE string from the callers buffer */
RtlInitEmptyUnicodeString(&WValue, ValueBuffer, ValueBufferLength);
- /* Convert the result to unicode */
+ /* Convert the result to UNICODE */
Status = RtlAnsiStringToUnicodeString(&WValue, &AValue, FALSE);
if (ReturnLength != NULL)
@@ -305,8 +301,7 @@
Status = STATUS_UNSUCCESSFUL;
}
- /* Cleanup allocated resources. */
- RtlFreeAnsiString(&AName);
+ /* Free the allocated ANSI value buffer */
ExFreePoolWithTag(AnsiValueBuffer, 'pmeT');
return Status;
7 Oct
7 Oct
9:24 a.m.
New subject: [ros-diffs] [hbelusca] 57504: [NTOSKRNL] Rearrange the NtQuerySystemEnvironmentValue code to have successive logical checks.
Hello !
If you want I will try to write a test ; however if I made this
rearrangement it's because it makes the code more "logic" : first of all
you
probe for read/write the arguments then you check whether or not you have
the right to continue (thus you don't start un-useful operation if you don't
have right to do so), then, in case of success, you initialize the variable
and AT THE VERY LAST, the user buffer just before filling it with the
information you gather.
Hermes
-----Message d'origine-----
De : ros-dev-bounces(a)reactos.org [mailto:ros-dev-bounces@reactos.org] De la
part de Timo Kreuzer
Envoyé : samedi 6 octobre 2012 22:21
À : ros-dev(a)reactos.org
Objet : Re: [ros-dev] [ros-diffs] [hbelusca] 57504: [NTOSKRNL] Rearrange the
NtQuerySystemEnvironmentValue code to have successive logical checks.
This changes the logic. I'm not sure how exact the previous version was, but
these changes should be tested.
Am 06.10.2012 21:50, schrieb hbelusca(a)svn.reactos.org:
============================================================================
==
_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
Author: hbelusca
Date: Sat Oct 6 19:50:17 2012
New Revision: 57504
URL: http://svn.reactos.org/svn/reactos?rev=57504&view=rev
Log:
[NTOSKRNL]
Rearrange the NtQuerySystemEnvironmentValue code to have successive
logical
checks.
Modified:
trunk/reactos/ntoskrnl/ex/sysinfo.c
Modified: trunk/reactos/ntoskrnl/ex/sysinfo.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ex/sysinfo.c?rev=5
7504&r1=57503&r2=57504&view=diff
--- trunk/reactos/ntoskrnl/ex/sysinfo.c [iso-8859-1]
(original)
+++ trunk/reactos/ntoskrnl/ex/sysinfo.c [iso-8859-1] Sat Oct 6 19:50:17
2012
@@ -235,44 +235,40 @@
_SEH2_YIELD(return _SEH2_GetExceptionCode());
}
_SEH2_END;
-
- }
-
- /* Allocate a buffer for the value */
- AnsiValueBuffer = ExAllocatePoolWithTag(NonPagedPool,
ValueBufferLength,
'pmeT');
- if (AnsiValueBuffer == NULL)
- {
- return STATUS_INSUFFICIENT_RESOURCES;
- }
-
- /*
- * Copy the name to kernel space if necessary and convert it to ANSI.
- */
+ }
+
+ /* According to NTInternals the SeSystemEnvironmentName privilege is
required!
*/
+ if
(!SeSinglePrivilegeCheck(SeSystemEnvironmentPrivilege,
PreviousMode))
+ {
+ DPRINT1("NtQuerySystemEnvironmentValue: Caller requires the
SeSystemEnvironmentPrivilege privilege!\n");
+ return STATUS_PRIVILEGE_NOT_HELD;
+ }
+
+ /* Copy the name to kernel space if necessary */
Status = ProbeAndCaptureUnicodeString(&WName, PreviousMode,
VariableName);
if (!NT_SUCCESS(Status))
{
return Status;
}
- /*
- * according to ntinternals the SeSystemEnvironmentName privilege is
required!
- */
- if (!SeSinglePrivilegeCheck(SeSystemEnvironmentPrivilege,
PreviousMode))
- {
- ReleaseCapturedUnicodeString(&WName, PreviousMode);
- DPRINT1("NtQuerySystemEnvironmentValue: Caller requires the
SeSystemEnvironmentPrivilege privilege!\n");
- return STATUS_PRIVILEGE_NOT_HELD;
- }
-
- /* Convert the value name to ansi and release the captured unicode
string */
+ /* Convert the name to ANSI and release the
captured UNICODE string
*/
Status =
RtlUnicodeStringToAnsiString(&AName, &WName, TRUE);
ReleaseCapturedUnicodeString(&WName, PreviousMode);
if (!NT_SUCCESS(Status)) return Status;
- /* Get the environment variable */
+ /* Allocate a buffer for the ANSI environment variable */
+ AnsiValueBuffer = ExAllocatePoolWithTag(NonPagedPool,
ValueBufferLength,
'pmeT');
+ if (AnsiValueBuffer == NULL)
+ {
+ RtlFreeAnsiString(&AName);
+ return STATUS_INSUFFICIENT_RESOURCES;
+ }
+
+ /* Get the environment variable and free the ANSI name */
Result = HalGetEnvironmentVariable(AName.Buffer,
(USHORT)ValueBufferLength,
AnsiValueBuffer);
+ RtlFreeAnsiString(&AName);
/* Check if we had success */
if (Result == ESUCCESS)
@@ -280,13 +276,13 @@
/* Copy the result back to the caller. */
_SEH2_TRY
{
- /* Initialize ansi string from the result */
+ /* Initialize ANSI string from the result */
RtlInitAnsiString(&AValue, AnsiValueBuffer);
- /* Initialize a unicode string from the callers buffer */
+ /* Initialize a UNICODE string from the callers buffer */
RtlInitEmptyUnicodeString(&WValue, ValueBuffer,
ValueBufferLength);
- /* Convert the result to unicode */
+ /* Convert the result to UNICODE */
Status = RtlAnsiStringToUnicodeString(&WValue, &AValue,
FALSE);
>
> if (ReturnLength != NULL)
> @@ -305,8 +301,7 @@
> Status = STATUS_UNSUCCESSFUL;
> }
>
> - /* Cleanup allocated resources. */
> - RtlFreeAnsiString(&AName);
> + /* Free the allocated ANSI value buffer */
> ExFreePoolWithTag(AnsiValueBuffer, 'pmeT');
>
> return Status;
1:32 p.m.
New subject: [ros-diffs] [hbelusca] 57504: [NTOSKRNL] Rearrange the NtQuerySystemEnvironmentValue code to have successive logical checks.
The question that you need to answer yourself is not whether something
is more logical or cleaner or whatever, but the question is: How does it
work on Windows.
For all internal APIs you can do as you like, as long as it doesn't
affect the externally visible behaviour. But in this case we have an
external API, which should
function 100% like the original (if possible)
Regards,
Timo
Am 07.10.2012 11:24, schrieb Hermès BÉLUSCA - MAÏTO:
Hello !
If you want I will try to write a test ; however if I made this
rearrangement it's because it makes the code more "logic" : first of all
you
probe for read/write the arguments then you check whether or not you have
the right to continue (thus you don't start un-useful operation if you don't
have right to do so), then, in case of success, you initialize the variable
and AT THE VERY LAST, the user buffer just before filling it with the
information you gather.
Hermes
-----Message d'origine-----
De : ros-dev-bounces(a)reactos.org [mailto:ros-dev-bounces@reactos.org] De la
part de Timo Kreuzer
Envoyé : samedi 6 octobre 2012 22:21
À : ros-dev(a)reactos.org
Objet : Re: [ros-dev] [ros-diffs] [hbelusca] 57504: [NTOSKRNL] Rearrange the
NtQuerySystemEnvironmentValue code to have successive logical checks.
This changes the logic. I'm not sure how exact the previous version was, but
these changes should be tested.
Am 06.10.2012 21:50, schrieb hbelusca(a)svn.reactos.org:
Author: hbelusca
Date: Sat Oct 6 19:50:17 2012
New Revision: 57504
URL: http://svn.reactos.org/svn/reactos?rev=57504&view=rev
Log:
[NTOSKRNL]
Rearrange the NtQuerySystemEnvironmentValue code to have successive
logical
checks.
Modified:
trunk/reactos/ntoskrnl/ex/sysinfo.c
Modified: trunk/reactos/ntoskrnl/ex/sysinfo.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ex/sysinfo.c?rev=5
7504&r1=57503&r2=57504&view=diff
============================================================================
==
--- trunk/reactos/ntoskrnl/ex/sysinfo.c
[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ex/sysinfo.c [iso-8859-1] Sat Oct 6 19:50:17
2012
@@ -235,44 +235,40 @@
_SEH2_YIELD(return _SEH2_GetExceptionCode());
}
_SEH2_END;
-
- }
-
- /* Allocate a buffer for the value */
- AnsiValueBuffer = ExAllocatePoolWithTag(NonPagedPool,
ValueBufferLength,
'pmeT');
- if (AnsiValueBuffer == NULL)
- {
- return STATUS_INSUFFICIENT_RESOURCES;
- }
-
- /*
- * Copy the name to kernel space if necessary and convert it to ANSI.
- */
+ }
+
+ /* According to NTInternals the SeSystemEnvironmentName privilege is
required! */
+ if
(!SeSinglePrivilegeCheck(SeSystemEnvironmentPrivilege,
PreviousMode))
+ {
+ DPRINT1("NtQuerySystemEnvironmentValue: Caller requires the
SeSystemEnvironmentPrivilege privilege!\n");
+ return STATUS_PRIVILEGE_NOT_HELD;
+ }
+
+ /* Copy the name to kernel space if necessary */
Status = ProbeAndCaptureUnicodeString(&WName, PreviousMode,
VariableName);
if (!NT_SUCCESS(Status))
{
return Status;
}
- /*
- * according to ntinternals the SeSystemEnvironmentName privilege is
required!
- */
- if (!SeSinglePrivilegeCheck(SeSystemEnvironmentPrivilege,
PreviousMode))
- {
- ReleaseCapturedUnicodeString(&WName, PreviousMode);
- DPRINT1("NtQuerySystemEnvironmentValue: Caller requires the
SeSystemEnvironmentPrivilege privilege!\n");
- return STATUS_PRIVILEGE_NOT_HELD;
- }
-
- /* Convert the value name to ansi and release the captured unicode
string */
+ /* Convert the name to ANSI and release the
captured UNICODE string
*/
Status =
RtlUnicodeStringToAnsiString(&AName, &WName, TRUE);
ReleaseCapturedUnicodeString(&WName, PreviousMode);
if (!NT_SUCCESS(Status)) return Status;
- /* Get the environment variable */
+ /* Allocate a buffer for the ANSI environment variable */
+ AnsiValueBuffer = ExAllocatePoolWithTag(NonPagedPool,
ValueBufferLength,
'pmeT');
+ if (AnsiValueBuffer == NULL)
+ {
+ RtlFreeAnsiString(&AName);
+ return STATUS_INSUFFICIENT_RESOURCES;
+ }
+
+ /* Get the environment variable and free the ANSI name */
Result = HalGetEnvironmentVariable(AName.Buffer,
(USHORT)ValueBufferLength,
AnsiValueBuffer);
+ RtlFreeAnsiString(&AName);
/* Check if we had success */
if (Result == ESUCCESS)
@@ -280,13 +276,13 @@
/* Copy the result back to the caller. */
_SEH2_TRY
{
- /* Initialize ansi string from the result */
+ /* Initialize ANSI string from the result */
RtlInitAnsiString(&AValue, AnsiValueBuffer);
- /* Initialize a unicode string from the callers buffer */
+ /* Initialize a UNICODE string from the callers buffer */
RtlInitEmptyUnicodeString(&WValue, ValueBuffer,
ValueBufferLength);
- /* Convert the result to unicode */
+ /* Convert the result to UNICODE */
Status = RtlAnsiStringToUnicodeString(&WValue, &AValue,
FALSE);
if (ReturnLength != NULL)
@@ -305,8 +301,7 @@
Status = STATUS_UNSUCCESSFUL;
}
- /* Cleanup allocated resources. */
- RtlFreeAnsiString(&AName);
+ /* Free the allocated ANSI value buffer */
ExFreePoolWithTag(AnsiValueBuffer, 'pmeT');
return Status;
_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
4426
days inactive
4427
days old
2 comments
2 participants
participants (2)
-
Hermès BÉLUSCA - MAÏTO -
Timo Kreuzer