Agreed..
When the assignment and assert are separated by a call
of X distance, the assert is certainly warranted.
Just not as it stands in ke_x.h
Perhaps you could factorize as suggested, Dmitri?
Best regards
L.
On 2016-09-26 10.28, ros-dev-request(a)reactos.org wrote:
> Rationale is simple:
>
> The real headers contain a KeEnterCriticalRegionThread function, which
> has this assert, and a KeEnterCriticalRegion function, which calls
> that function with KeGetCurrentThread() at input. This is an
> optimization such that if you already have the current thread of
> interest somewhere, you can call KeEnterCriticalRegionThread and avoid
> reading from FS or GS again. ReactOS combines these two functions in
> one, and as such the ASSERT appears pointless. Factoring the functions
> would've been the correct approach -- not removing the ASSERT.
>
> Best regards,
> Alex Ionescu
--
There is one thing stronger than all the armies in the world,
and that's an idea whose time has come. [Victor Hugo]
Rationale is simple:
The real headers contain a KeEnterCriticalRegionThread function, which has
this assert, and a KeEnterCriticalRegion function, which calls that
function with KeGetCurrentThread() at input. This is an optimization such
that if you already have the current thread of interest somewhere, you can
call KeEnterCriticalRegionThread and avoid reading from FS or GS again.
ReactOS combines these two functions in one, and as such the ASSERT appears
pointless. Factoring the functions would've been the correct approach --
not removing the ASSERT.
Best regards,
Alex Ionescu
On Sun, Sep 25, 2016 at 8:13 PM, Love Nystrom <love.nystrom(a)gmail.com>
wrote:
> I don't want to trod on anyone's tail, but
> assigning _Thread = KeGetCurrentThread(), and
> then asserting _Thread == KeGetCurrentThread()
> on the next line does seem excessively myopic, yes ?
>
> Unless you expect KeGetCurrentThread() to be fubar
> and return rand(), in which case the assert should be removed
> when KeGetCurrentThread() gets it's head on straight.
>
> As to the talent, or lack thereof, of MS programmers,
> we might talk about it if we ever have a table with some
> beer on it between us.
>
> They're not *all* wrong.. no ;)
> L.
>
> On 2016-09-24 23.23, ros-dev-request(a)reactos.org wrote:
>
> Subject:
> Re: [ros-dev] [ros-diffs] [dchapyshev] 72787: [NTOSKRNL] Remove unneeded
> sanity checks
>
> From:
> Alex Ionescu <ionucu(a)videotron.ca> <ionucu(a)videotron.ca>
>
> Date:
> 2016-09-24 21.29
>
> To:
> ReactOS Development List <ros-dev(a)reactos.org> <ros-dev(a)reactos.org>
>
> CC:
> Linda Wang <ros-diffs(a)reactos.org> <ros-diffs(a)reactos.org>
>
> Thanks for removing stuff that exists in the NT kernel as sanity
> checks -- the entire MS dev team must be wrong, thanks for correcting
> them all :)
>
> Make sure not to ask "anyone can explain these checks? they seem
> useless to me" when removing stuff like this.
> Best regards,
> Alex Ionescu
>
>
> On Sat, Sep 24, 2016 at 2:30 AM, <dchapyshev(a)svn.reactos.org> <dchapyshev(a)svn.reactos.org> wrote:
>
> > Author: dchapyshev> Date: Sat Sep 24 09:30:06 2016> New Revision: 72787>> URL: http://svn.reactos.org/svn/reactos?rev=72787&view=rev> Log:> [NTOSKRNL] Remove unneeded sanity checks>> Modified:> trunk/reactos/ntoskrnl/include/internal/ke_x.h>> Modified: trunk/reactos/ntoskrnl/include/internal/ke_x.h> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…> ==============================================================================> --- trunk/reactos/ntoskrnl/include/internal/ke_x.h [iso-8859-1] (original)> +++ trunk/reactos/ntoskrnl/include/internal/ke_x.h [iso-8859-1] Sat Sep 24 09:30:06 2016> @@ -25,7 +25,6 @@> \> /* Sanity checks */ \> ASSERT(KeGetCurrentIrql() <= APC_LEVEL); \> - ASSERT(_Thread == KeGetCurrentThread()); \> ASSERT((_Thread->SpecialApcDisable <= 0) && \> (_Thread->SpecialApcDisable != -32768)); \> \> @@ -42,7 +41,6 @@> \> /* Sanity checks */ \> ASSERT(KeGetCurrentIrql() <= APC_LEVEL); \> - ASSERT(_Thread == KeGetCurrentThread()); \> ASSERT(_Thread->SpecialApcDisable < 0); \> \> /* Leave region and check if APCs are OK now */ \> @@ -66,7 +64,6 @@> PKTHREAD _Thread = KeGetCurrentThread(); \> \> /* Sanity checks */ \> - ASSERT(_Thread == KeGetCurrentThread()); \> ASSERT((_Thread->KernelApcDisable <= 0) && \> (_Thread->KernelApcDisable != -32768)); \> \> @@ -82,7 +79,6 @@> PKTHREAD _Thread = KeGetCurrentThread(); \> \> /* Sanity checks */ \> - ASSERT(_Thread == KeGetCurrentThread()); \> ASSERT(_Thread->KernelApcDisable < 0); \> \> /* Enable Kernel APCs */ \>>
>
>
> --
> There is one thing stronger than all the armies in the world,
> and that's an idea whose time has come. [Victor Hugo]
>
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
>
>
I don't want to trod on anyone's tail, but
assigning _Thread = KeGetCurrentThread(), and
then asserting _Thread == KeGetCurrentThread()
on the next line does seem excessively myopic, yes ?
Unless you expect KeGetCurrentThread() to be fubar
and return rand(), in which case the assert should be removed
when KeGetCurrentThread() gets it's head on straight.
As to the talent, or lack thereof, of MS programmers,
we might talk about it if we ever have a table with some
beer on it between us.
They're not *all* wrong.. no ;)
L.
On 2016-09-24 23.23, ros-dev-request(a)reactos.org wrote:
> Subject:
> Re: [ros-dev] [ros-diffs] [dchapyshev] 72787: [NTOSKRNL] Remove
> unneeded sanity checks
> From:
> Alex Ionescu <ionucu(a)videotron.ca>
> Date:
> 2016-09-24 21.29
>
> To:
> ReactOS Development List <ros-dev(a)reactos.org>
> CC:
> Linda Wang <ros-diffs(a)reactos.org>
>
>
> Thanks for removing stuff that exists in the NT kernel as sanity
> checks -- the entire MS dev team must be wrong, thanks for correcting
> them all:)
>
> Make sure not to ask "anyone can explain these checks? they seem
> useless to me" when removing stuff like this.
> Best regards,
> Alex Ionescu
>
>
> On Sat, Sep 24, 2016 at 2:30 AM,<dchapyshev(a)svn.reactos.org> wrote:
>> >Author: dchapyshev
>> >Date: Sat Sep 24 09:30:06 2016
>> >New Revision: 72787
>> >
>> >URL:http://svn.reactos.org/svn/reactos?rev=72787&view=rev
>> >Log:
>> >[NTOSKRNL] Remove unneeded sanity checks
>> >
>> >Modified:
>> > trunk/reactos/ntoskrnl/include/internal/ke_x.h
>> >
>> >Modified: trunk/reactos/ntoskrnl/include/internal/ke_x.h
>> >URL:http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/inter…
>> >==============================================================================
>> >--- trunk/reactos/ntoskrnl/include/internal/ke_x.h [iso-8859-1] (original)
>> >+++ trunk/reactos/ntoskrnl/include/internal/ke_x.h [iso-8859-1] Sat Sep 24 09:30:06 2016
>> >@@ -25,7 +25,6 @@
>> > \
>> > /* Sanity checks */ \
>> > ASSERT(KeGetCurrentIrql() <= APC_LEVEL); \
>> >- ASSERT(_Thread == KeGetCurrentThread()); \
>> > ASSERT((_Thread->SpecialApcDisable <= 0) && \
>> > (_Thread->SpecialApcDisable != -32768)); \
>> > \
>> >@@ -42,7 +41,6 @@
>> > \
>> > /* Sanity checks */ \
>> > ASSERT(KeGetCurrentIrql() <= APC_LEVEL); \
>> >- ASSERT(_Thread == KeGetCurrentThread()); \
>> > ASSERT(_Thread->SpecialApcDisable < 0); \
>> > \
>> > /* Leave region and check if APCs are OK now */ \
>> >@@ -66,7 +64,6 @@
>> > PKTHREAD _Thread = KeGetCurrentThread(); \
>> > \
>> > /* Sanity checks */ \
>> >- ASSERT(_Thread == KeGetCurrentThread()); \
>> > ASSERT((_Thread->KernelApcDisable <= 0) && \
>> > (_Thread->KernelApcDisable != -32768)); \
>> > \
>> >@@ -82,7 +79,6 @@
>> > PKTHREAD _Thread = KeGetCurrentThread(); \
>> > \
>> > /* Sanity checks */ \
>> >- ASSERT(_Thread == KeGetCurrentThread()); \
>> > ASSERT(_Thread->KernelApcDisable < 0); \
>> > \
>> > /* Enable Kernel APCs */ \
>> >
>> >
--
There is one thing stronger than all the armies in the world,
and that's an idea whose time has come. [Victor Hugo]
Don't change code when runtime conditions evolve.Just give more RAM to the bot and it's over.
Envoyé depuis Yahoo Mail pour Android
Le jeu. j sept. PM à 18:01, Thomas Faber (JIRA)<noreply(a)reactos.org> a écrit :
[ https://jira.reactos.org/browse/CORE-12020?page=com.atlassian.jira.plugin.s… ]
Thomas Faber commented on CORE-12020:
-------------------------------------
First stage is extremely heavy on memory due to inf parsing. One thing that helped in a similar situation was to separate caroots.inf from registry.inf and have it parsed separately. We should probably do that in trunk...
> We're not able to perform DPH test runs anymore
> -----------------------------------------------
>
> Key: CORE-12020
> URL: https://jira.reactos.org/browse/CORE-12020
> Project: Core ReactOS
> Issue Type: Bug
> Reporter: Amine Khaldi
> Assignee: Bug Zilla
> Labels: REGRESSION
>
> https://build.reactos.org/builders/Test%20KVM/builds/15147/steps/test/logs/…
> https://jira.reactos.org/secure/attachment/37125/ was used with patchbot.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
_______________________________________________
Ros-bugs mailing list
Ros-bugs(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-bugs