pschweitzer(a)svn.reactos.org wrote:
> +DWORD WINAPI sfc_8()
>
I hope you guys realize that sfc_8 () compiled as C is different from
what it'd be when compiled as C++. In C this means that it is a cdecl
function with variable arguments (I guess WINAPI overrides cdecl, but
still). If it is compiled as C++, it means the same as sfc_8 (void). So
better have a void argument list.
Thomas
sginsberg(a)svn.reactos.org a écrit :
> Author: sginsberg
> Date: Sun Aug 24 10:48:05 2008
> New Revision: 35600
>
> URL: http://svn.reactos.org/svn/reactos?rev=35600&view=rev
> Log:
> - Remove KEBUGCHECK and KEBUGCHECKEX macros
> - Replace "KeBugCheck(0)" by ASSERT(FALSE)
> - Replace deprecated "CPRINT" by DRINT1
>
You probably understand that KEBUGCHECK(0) is *NOT* the same as
ASSERT(FALSE)
ASSERT(FALSE) is valid only in debug mode, and lets the code continue in
release mode.
KEBUGCHECK stops the execution also in release mode.
For example, in ntoskrnl/dbgk/dbgkobj.c, code now continues even if
DbgkpQueueMessage failed. Are you sure it is safe to continue in that case?
(same question applies for other locations)
Hervé
In the kernel, stdcall IoCallDriver is macroed to the fastcall IofCallDriver. However, for clarification I think that IoCallDriver should be used, and this is what we do everywhere else.
>Why this?
>> ======================================================================
>> --- trunk/reactos/ntoskrnl/io/iomgr/iofunc.c [iso-8859-1] (original)
>> +++ trunk/reactos/ntoskrnl/io/iomgr/iofunc.c [iso-8859-1] Sat Aug 23
>> 11:30:14 2008
>> @@ -682,7 +682,7 @@
>> StackPtr->FileObject = FileObject;
>>
>> /* Call the Driver */
>> - return IofCallDriver(DeviceObject, Irp);
>> + return IoCallDriver(DeviceObject, Irp);
>> }
>>
>> /*
>> @@ -732,7 +732,7 @@
>> StackPtr->FileObject = FileObject;
>>
>> /* Call the Driver */
>> - return IofCallDriver(DeviceObject, Irp);
>> + return IoCallDriver(DeviceObject, Irp);
>> }
>>
>>
_________________________________________________________________
Discover the new Windows Vista
http://search.msn.com/results.aspx?q=windows+vista&mkt=en-US&form=QBRE
--- Please, please, don't think I'm trying to say "revert this!" ---
There are dozens (if not more) places in the kernel where such checks
could/should be made. I believe the more correct approach instead of
adding them to every function, is to use a similar framework to
Microsoft's Driver Verifier, where such functions are hooked and
wrapped through similar verifications.
I'm not saying this particular assert should be removed, I'm just
letting people know that if there's going to be major work on this --
there's a better way.
Just my 2c.
On 23-Aug-08, at 4:04 PM, arty(a)svn.reactos.org wrote:
> Author: arty
> Date: Sat Aug 23 18:04:15 2008
> New Revision: 35581
>
> URL: http://svn.reactos.org/svn/reactos?rev=35581&view=rev
> Log:
> Catch failure to release the cancel spinlock. Would've spotted my
> error in
> tcpip.sys.
>
> Modified:
> trunk/reactos/ntoskrnl/io/iomgr/irp.c
>
> Modified: trunk/reactos/ntoskrnl/io/iomgr/irp.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/irp.c?re…
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/reactos/ntoskrnl/io/iomgr/irp.c [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/io/iomgr/irp.c [iso-8859-1] Sat Aug 23
> 18:04:15 2008
> @@ -970,12 +970,14 @@
> IoCancelIrp(IN PIRP Irp)
> {
> KIRQL OldIrql;
> + KIRQL IrqlAtEntry;
> PDRIVER_CANCEL CancelRoutine;
> IOTRACE(IO_IRP_DEBUG,
> "%s - Canceling IRP %p\n",
> __FUNCTION__,
> Irp);
> ASSERT(Irp->Type == IO_TYPE_IRP);
> + IrqlAtEntry = KeGetCurrentIrql();
>
> /* Acquire the cancel lock and cancel the IRP */
> IoAcquireCancelSpinLock(&OldIrql);
> @@ -999,6 +1001,7 @@
> /* Set the cancel IRQL And call the routine */
> Irp->CancelIrql = OldIrql;
> CancelRoutine(IoGetCurrentIrpStackLocation(Irp)-
> >DeviceObject, Irp);
> + ASSERT(IrqlAtEntry == KeGetCurrentIrql());
> return TRUE;
> }
>
>
Best regards,
Alex Ionescu
