Ros-dev September 2016

ros-dev@reactos.org

17 participants
24 discussions

Re: [ros-dev] KD trace during ROS installation

by Love Nystrom

Good afternoon gentlemen... Seems the list server has gone on a vacation. Still waiting on the digest where Hermes added to Thomas answer.. Anyway Hermes pasted his answer to my mailbox (thank you) and so.. One more step is required in addition to the answers from Thomas and Hermes.. A small patch in 'base/setup/usetup/bootsup.c', which contains the code that writes 'freeldr.ini' to the HD during install. Without patching in the desired COM port there, trace is lost after ROS installation reboots from HD. Perhaps we should have a wiki page to cover this ? And/or perhaps we should make it easier to make this mod, by moving the hard coded strings from bootsup.c into an ini file or whatever ? Anyway, I got my install trace, and posted a clip concerning the Gecko breakage to CORE-12011. Best Regards L.

8 years, 2 months

Status Meeting (September 2016)

by Aleksey Bragin

Hello, Let me invite you to the monthly status meeting taking place this Thursday, 29th of September, 19:00 UTC. IRC service will only be started shortly before the meeting. Your participation passwords and server address will be emailed to you shortly before the meeting starts, and they are going to be different once again as they are not stored in any database. Hopefully it's not much of inconvenience. Or we'll do it in #reactos-meeting on Freenode, depeonds on our admins. Please send agenda proposals to me before the meeting so we don't waste time trying to setup the agenda directly during the meeting. Regards, Aleksey Bragin

8 years, 2 months

Re: [ros-dev] [ros-diffs] [dchapyshev] 72835: [NTOS:FSRTL] Rework FsRtlIsNameInExpressionPrivate for correct parsing some expressions * Fixes 1 test for kmtest:FsRtlExpression and 15 tests for kernel32:file

by Pierre Schweitzer

Don't forget to apply the changes (not reviewed, sorry, no time) to FsRtlIsDbcsInExpression. Le 28/09/2016 à 01:00, dchapyshev(a)svn.reactos.org a écrit : > Author: dchapyshev > Date: Tue Sep 27 23:00:20 2016 > New Revision: 72835 > > URL: http://svn.reactos.org/svn/reactos?rev=72835&view=rev > Log: > [NTOS:FSRTL] Rework FsRtlIsNameInExpressionPrivate for correct parsing some expressions > > * Fixes 1 test for kmtest:FsRtlExpression and 15 tests for kernel32:file > > Modified: > trunk/reactos/ntoskrnl/fsrtl/name.c > > Modified: trunk/reactos/ntoskrnl/fsrtl/name.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/fsrtl/name.c?rev=… > ============================================================================== > --- trunk/reactos/ntoskrnl/fsrtl/name.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/fsrtl/name.c [iso-8859-1] Tue Sep 27 23:00:20 2016 > @@ -23,13 +23,14 @@ > IN BOOLEAN IgnoreCase, > IN PWCHAR UpcaseTable OPTIONAL) > { > - SHORT StarFound = -1, DosStarFound = -1; > - USHORT BackTrackingBuffer[5], DosBackTrackingBuffer[5]; > - PUSHORT BackTracking = BackTrackingBuffer, DosBackTracking = DosBackTrackingBuffer; > - SHORT BackTrackingSize = RTL_NUMBER_OF(BackTrackingBuffer); > - SHORT DosBackTrackingSize = RTL_NUMBER_OF(DosBackTrackingBuffer); > + USHORT Offset, Position, BackTrackingPosition, OldBackTrackingPosition; > + USHORT BackTrackingBuffer[16], OldBackTrackingBuffer[16] = {0}; > + PUSHORT BackTrackingSwap, BackTracking = BackTrackingBuffer, OldBackTracking = OldBackTrackingBuffer; > UNICODE_STRING IntExpression; > - USHORT ExpressionPosition = 0, NamePosition = 0, MatchingChars, LastDot; > + USHORT ExpressionPosition = 0, NamePosition = 0, MatchingChars = 1; > + BOOLEAN EndOfName = FALSE; > + BOOLEAN Result = FALSE; > + BOOLEAN DontSkipDot; > WCHAR CompareChar; > PAGED_CODE(); > > @@ -37,7 +38,7 @@ > if (!Name->Length || !Expression->Length) > { > /* Return TRUE if both strings are empty, otherwise FALSE */ > - if (Name->Length == 0 && Expression->Length == 0) > + if (!Name->Length && !Expression->Length) > return TRUE; > else > return FALSE; > @@ -103,193 +104,144 @@ > } > } > > - while ((NamePosition < Name->Length / sizeof(WCHAR)) && > - (ExpressionPosition < Expression->Length / sizeof(WCHAR))) > - { > - /* Basic check to test if chars are equal */ > - CompareChar = IgnoreCase ? UpcaseTable[Name->Buffer[NamePosition]] : > - Name->Buffer[NamePosition]; > - if (Expression->Buffer[ExpressionPosition] == CompareChar) > - { > - NamePosition++; > - ExpressionPosition++; > - } > - /* Check cases that eat one char */ > - else if (Expression->Buffer[ExpressionPosition] == L'?') > - { > - NamePosition++; > - ExpressionPosition++; > - } > - /* Test star */ > - else if (Expression->Buffer[ExpressionPosition] == L'*') > - { > - /* Skip contigous stars */ > - while ((ExpressionPosition + 1 < (USHORT)(Expression->Length / sizeof(WCHAR))) && > - (Expression->Buffer[ExpressionPosition + 1] == L'*')) > - { > - ExpressionPosition++; > - } > - > - /* Save star position */ > - StarFound++; > - if (StarFound >= BackTrackingSize) > - { > - ASSERT(BackTracking == BackTrackingBuffer); > - > - BackTrackingSize = Expression->Length / sizeof(WCHAR); > - BackTracking = ExAllocatePoolWithTag(PagedPool | POOL_RAISE_IF_ALLOCATION_FAILURE, > - BackTrackingSize * sizeof(USHORT), > - 'nrSF'); > - RtlCopyMemory(BackTracking, BackTrackingBuffer, sizeof(BackTrackingBuffer)); > - > - } > - BackTracking[StarFound] = ExpressionPosition++; > - > - /* If star is at the end, then eat all rest and leave */ > - if (ExpressionPosition == Expression->Length / sizeof(WCHAR)) > - { > - NamePosition = Name->Length / sizeof(WCHAR); > + /* Name parsing loop */ > + for (; !EndOfName; MatchingChars = BackTrackingPosition, NamePosition++) > + { > + /* Reset positions */ > + OldBackTrackingPosition = BackTrackingPosition = 0; > + > + if (NamePosition >= Name->Length / sizeof(WCHAR)) > + { > + EndOfName = TRUE; > + if (OldBackTracking[MatchingChars - 1] == Expression->Length * 2) > break; > - } > - > - /* Allow null matching */ > - if (Expression->Buffer[ExpressionPosition] != L'?' && > - Expression->Buffer[ExpressionPosition] != Name->Buffer[NamePosition]) > - { > - NamePosition++; > - } > - } > - /* Check DOS_STAR */ > - else if (Expression->Buffer[ExpressionPosition] == DOS_STAR) > - { > - /* Skip contigous stars */ > - while ((ExpressionPosition + 1 < (USHORT)(Expression->Length / sizeof(WCHAR))) && > - (Expression->Buffer[ExpressionPosition + 1] == DOS_STAR)) > - { > - ExpressionPosition++; > - } > - > - /* Look for last dot */ > - MatchingChars = 0; > - LastDot = (USHORT)-1; > - while (MatchingChars < Name->Length / sizeof(WCHAR)) > - { > - if (Name->Buffer[MatchingChars] == L'.') > - { > - LastDot = MatchingChars; > - if (LastDot > NamePosition) > - break; > - } > - > - MatchingChars++; > - } > - > - /* If we don't have dots or we didn't find last yet > - * start eating everything > - */ > - if (MatchingChars != Name->Length || LastDot == (USHORT)-1) > - { > - DosStarFound++; > - if (DosStarFound >= DosBackTrackingSize) > - { > - ASSERT(DosBackTracking == DosBackTrackingBuffer); > - > - DosBackTrackingSize = Expression->Length / sizeof(WCHAR); > - DosBackTracking = ExAllocatePoolWithTag(PagedPool | POOL_RAISE_IF_ALLOCATION_FAILURE, > - DosBackTrackingSize * sizeof(USHORT), > + } > + > + while (MatchingChars > OldBackTrackingPosition) > + { > + ExpressionPosition = (OldBackTracking[OldBackTrackingPosition++] + 1) / 2; > + > + /* Expression parsing loop */ > + for (Offset = 0; ExpressionPosition < Expression->Length; Offset = sizeof(WCHAR)) > + { > + ExpressionPosition += Offset; > + > + if (ExpressionPosition == Expression->Length) > + { > + BackTracking[BackTrackingPosition++] = Expression->Length * 2; > + break; > + } > + > + /* If buffer too small */ > + if (BackTrackingPosition > RTL_NUMBER_OF(BackTrackingBuffer) - 1) > + { > + /* Allocate memory for BackTracking */ > + BackTracking = ExAllocatePoolWithTag(PagedPool | POOL_RAISE_IF_ALLOCATION_FAILURE, > + (Expression->Length + sizeof(WCHAR)) * sizeof(USHORT), > + 'nrSF'); > + /* Copy old buffer content */ > + RtlCopyMemory(BackTracking, > + BackTrackingBuffer, > + RTL_NUMBER_OF(BackTrackingBuffer) * sizeof(USHORT)); > + > + /* Allocate memory for OldBackTracking */ > + OldBackTracking = ExAllocatePoolWithTag(PagedPool | POOL_RAISE_IF_ALLOCATION_FAILURE, > + (Expression->Length + sizeof(WCHAR)) * sizeof(USHORT), > 'nrSF'); > - RtlCopyMemory(DosBackTracking, DosBackTrackingBuffer, sizeof(DosBackTrackingBuffer)); > - } > - DosBackTracking[DosStarFound] = ExpressionPosition++; > - > - /* Not the same char, start exploring */ > - if (Expression->Buffer[ExpressionPosition] != Name->Buffer[NamePosition]) > - NamePosition++; > - } > - else > - { > - /* Else, if we are at last dot, eat it - otherwise, null match */ > - if (Name->Buffer[NamePosition] == '.') > - NamePosition++; > - > - ExpressionPosition++; > - } > - } > - /* Check DOS_DOT */ > - else if (Expression->Buffer[ExpressionPosition] == DOS_DOT) > - { > - /* We only match dots */ > - if (Name->Buffer[NamePosition] == L'.') > - { > - NamePosition++; > - } > - /* Try to explore later on for null matching */ > - else if ((ExpressionPosition + 1 < (USHORT)(Expression->Length / sizeof(WCHAR))) && > - (Name->Buffer[NamePosition] == Expression->Buffer[ExpressionPosition + 1])) > - { > - NamePosition++; > - } > - ExpressionPosition++; > - } > - /* Check DOS_QM */ > - else if (Expression->Buffer[ExpressionPosition] == DOS_QM) > - { > - /* We match everything except dots */ > - if (Name->Buffer[NamePosition] != L'.') > - { > - NamePosition++; > - } > - ExpressionPosition++; > - } > - /* If nothing match, try to backtrack */ > - else if (StarFound >= 0) > - { > - ExpressionPosition = BackTracking[StarFound--]; > - } > - else if (DosStarFound >= 0) > - { > - ExpressionPosition = DosBackTracking[DosStarFound--]; > - } > - /* Otherwise, fail */ > - else > - { > - break; > - } > - > - /* Under certain circumstances, expression is over, but name isn't > - * and we can backtrack, then, backtrack */ > - if (ExpressionPosition == Expression->Length / sizeof(WCHAR) && > - NamePosition != Name->Length / sizeof(WCHAR) && > - StarFound >= 0) > - { > - ExpressionPosition = BackTracking[StarFound--]; > - } > - } > - /* If we have nullable matching wc at the end of the string, eat them */ > - if (ExpressionPosition != Expression->Length / sizeof(WCHAR) && NamePosition == Name->Length / sizeof(WCHAR)) > - { > - while (ExpressionPosition < Expression->Length / sizeof(WCHAR)) > - { > - if (Expression->Buffer[ExpressionPosition] != DOS_DOT && > - Expression->Buffer[ExpressionPosition] != L'*' && > - Expression->Buffer[ExpressionPosition] != DOS_STAR) > - { > + /* Copy old buffer content */ > + RtlCopyMemory(OldBackTracking, > + OldBackTrackingBuffer, > + RTL_NUMBER_OF(OldBackTrackingBuffer) * sizeof(USHORT)); > + } > + > + /* Basic check to test if chars are equal */ > + CompareChar = IgnoreCase ? UpcaseTable[Name->Buffer[NamePosition]] : > + Name->Buffer[NamePosition]; > + if (Expression->Buffer[ExpressionPosition / sizeof(WCHAR)] == CompareChar && !EndOfName) > + { > + BackTracking[BackTrackingPosition++] = (ExpressionPosition + sizeof(WCHAR)) * 2; > + } > + /* Check cases that eat one char */ > + else if (Expression->Buffer[ExpressionPosition / sizeof(WCHAR)] == L'?' && !EndOfName) > + { > + BackTracking[BackTrackingPosition++] = (ExpressionPosition + sizeof(WCHAR)) * 2; > + } > + /* Test star */ > + else if (Expression->Buffer[ExpressionPosition / sizeof(WCHAR)] == L'*') > + { > + BackTracking[BackTrackingPosition++] = ExpressionPosition * 2; > + BackTracking[BackTrackingPosition++] = (ExpressionPosition * 2) + 3; > + continue; > + } > + /* Check DOS_STAR */ > + else if (Expression->Buffer[ExpressionPosition / sizeof(WCHAR)] == DOS_STAR) > + { > + /* Look for last dot */ > + DontSkipDot = TRUE; > + if (!EndOfName && Name->Buffer[NamePosition] == '.') > + { > + for (Position = NamePosition - 1; Position < Name->Length; Position++) > + { > + if (Name->Buffer[Position] == L'.') > + { > + DontSkipDot = FALSE; > + break; > + } > + } > + } > + > + if (EndOfName || Name->Buffer[NamePosition] != L'.' || !DontSkipDot) > + BackTracking[BackTrackingPosition++] = ExpressionPosition * 2; > + > + BackTracking[BackTrackingPosition++] = (ExpressionPosition * 2) + 3; > + continue; > + } > + /* Check DOS_DOT */ > + else if (Expression->Buffer[ExpressionPosition / sizeof(WCHAR)] == DOS_DOT) > + { > + if (EndOfName) continue; > + > + if (Name->Buffer[NamePosition] == L'.') > + BackTracking[BackTrackingPosition++] = (ExpressionPosition + sizeof(WCHAR)) * 2; > + } > + /* Check DOS_QM */ > + else if (Expression->Buffer[ExpressionPosition / sizeof(WCHAR)] == DOS_QM) > + { > + if (EndOfName || Name->Buffer[NamePosition] == L'.') continue; > + > + BackTracking[BackTrackingPosition++] = (ExpressionPosition + sizeof(WCHAR)) * 2; > + } > + > + /* Leave from loop */ > break; > } > - ExpressionPosition++; > - } > - } > - > - if (BackTracking != BackTrackingBuffer) > - { > + > + for (Position = 0; MatchingChars > OldBackTrackingPosition && Position < BackTrackingPosition; Position++) > + { > + while (MatchingChars > OldBackTrackingPosition && > + BackTracking[Position] > OldBackTracking[OldBackTrackingPosition]) > + { > + ++OldBackTrackingPosition; > + } > + } > + } > + > + /* Swap pointers */ > + BackTrackingSwap = BackTracking; > + BackTracking = OldBackTracking; > + OldBackTracking = BackTrackingSwap; > + } > + > + /* Store result value */ > + Result = (OldBackTracking[MatchingChars - 1] == (Expression->Length * 2)); > + > + /* Frees the memory if necessary */ > + if (BackTracking != BackTrackingBuffer && BackTracking != OldBackTrackingBuffer) > ExFreePoolWithTag(BackTracking, 'nrSF'); > - } > - if (DosBackTracking != DosBackTrackingBuffer) > - { > - ExFreePoolWithTag(DosBackTracking, 'nrSF'); > - } > - > - return (ExpressionPosition == Expression->Length / sizeof(WCHAR) && NamePosition == Name->Length / sizeof(WCHAR)); > + if (OldBackTracking != BackTrackingBuffer && OldBackTracking != OldBackTrackingBuffer) > + ExFreePoolWithTag(OldBackTracking, 'nrSF'); > + > + return Result; > } > > /* PUBLIC FUNCTIONS **********************************************************/ > > -- Pierre Schweitzer <pierre at reactos.org> System & Network Administrator Senior Kernel Developer ReactOS Deutschland e.V.

8 years, 2 months

Re: [ros-dev] KD trace during ROS installation

by Love Nystrom

Thanks Thomas, Yes and No... I added Options=/DEBUG /DEBUGPORT=COM2 /BAUDRATE=115200 /SOS / KDSERIAL which ended up on the BootCD all right, but it seems KD is never brought in, or doesn't use the settings from freeldr.ini on the BootCD, so no trace :/ (Ok, /SOS was perhaps a bit over the top, but anyway.) Do you know where the source is for what becomes 'freeldr.ini' on the HD after installation? If I can modify that, I can at least get a trace on the later phases of installation, after ROS boots from HD. Then I might get a hint on why the Wine Gecko installer is broken. (It waits forever for whatever it might be.. Lucky it can be canceled.) Speaking of which, shouldn't the Wine Gecko installer be included in the RosApps list so one can install it later? ATM it isn't. Best Regards L. On 2016-09-27 19.00, Thomas Faber <thomas.faber(a)reactos.org> wrote: > You should be able to simply add an > Options=... > line to the [Setup] section in boot/bootdata/bootcd.ini to modify > the setup's freeldr options. That file becomes freeldr.ini in the root > dir of the BootCD. > > > On 2016-09-27 12:21, Love Nystrom wrote: >> >Good afternoon gentlemen.. >> > >> >Can anyone tell me at what point during ROS installation KD becomes >> >operative? >> >And if there is a way to change the default KD COM port from COM1 to COM2 >> >in order to trace KD output during installation? >> > >> >(I run my ROS tests on VMware with a com0com connection from ReactOS COM2 >> >to my host machine, which works fine after install is complete and I can >> >edit freeldr.ini, >> >but I'd like to be able to collect KD output during installation as well.) >> > >> >(COM1 is not an option, since it's a physical port on both host and guest.) >> > >> >Do I have to patch KD itself ? >> > >> >Best Regards >> >L. -- There is one thing stronger than all the armies in the world, and that's an idea whose time has come. [Victor Hugo]

8 years, 2 months

KD trace during ROS installation

by Love Nystrom

Good afternoon gentlemen.. Can anyone tell me at what point during ROS installation KD becomes operative? And if there is a way to change the default KD COM port from COM1 to COM2 in order to trace KD output during installation? (I run my ROS tests on VMware with a com0com connection from ReactOS COM2 to my host machine, which works fine after install is complete and I can edit freeldr.ini, but I'd like to be able to collect KD output during installation as well.) (COM1 is not an option, since it's a physical port on both host and guest.) Do I have to patch KD itself ? Best Regards L.

8 years, 2 months

Re: [ros-dev] [ros-diffs] [dchapyshev] 72787: [NTOSKRNL] Remove unneeded sanity checks

by Love Nystrom

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]

8 years, 3 months

Re: [ros-dev] Ros-dev Digest, Vol 145, Issue 16

by Love Nystrom

8 years, 3 months

Re: [ros-dev] [ros-diffs] [dchapyshev] 72787: [NTOSKRNL] Remove unneeded sanity checks

by Alex Ionescu

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 > >

8 years, 3 months

Re: [ros-dev] [ros-diffs] [dchapyshev] 72787: [NTOSKRNL] Remove unneeded sanity checks

by Love Nystrom

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]

8 years, 3 months

Re: [ros-dev] [ros-diffs] [dchapyshev] 72787: [NTOSKRNL] Remove unneeded sanity checks

by Alex Ionescu

There are no disadvantages. Someone though it would be fun to remove useful assertions because they don't really understand them. Best regards, Alex Ionescu On Sat, Sep 24, 2016 at 9:23 AM, Michael Fritscher <michael(a)fritscher.net> wrote: > Hi, > > hmm, what are disadvantages of these checks? If MS uses them as well these > shouldn't break anything. > > Perfomancewise there shouldn't be a difference, and in release builds they > are disabled anyways, aren't they? > > I only want to get a better unterstanding ;-) > > Best regards, > Michael > >> 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/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 */ >>> \ >>> >>> >> >> _______________________________________________ >> 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

8 years, 3 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-dev September 2016