- Ros-dev - ReactOS.org

Re: [ros-dev] [ros-diffs] [dchapyshev] 72655: [NtUser] Try to fix tests for user32:EnumDisplaySettings
by Thomas Faber 11 Sep '16

11 Sep '16

I've seen like 4 people change this stuff recently and not one of them got it 100% right. So here are the rules for how to use probes and SEH: * Probe pointers before accessing them. This ensures you're not reading kernel mode memory (and does pretty much nothing else) * Never access user mode pointers outside of SEH. The application controls its address space, so protection, physical memory mapping and contents can change out from under you at any point. For most purposes, MmProbeAndLockPages is the only way to get around this. * Sometimes there is no alternative to copying data to kernel memory. If there is a pointer stored in user mode memory, you need a copy of it because you can't rely on it not changing after your probe (even if you lock down the memory). * In case of an exception you usually want to return the exception code. There are exceptions (heh) to this rule, but if you think you've found one, better be sure and show it with a test. * If you make a pool allocation whose size is directly influenced by user mode input, there's a good chance you'll want to use ExAllocatePoolWithQuotaTag In particular this means: * Probing something or accessing it once does not save you from using SEH every single time you access the user mode memory * Probing UserModePointer->StructMemberPointer is never correct, you need to make a copy of the struct or that particular member, since the memory can change * The ProbeForReadUnicodeString function is not useful unless you use its return value Hope this helps. Thanks for hanging in there. -T On 2016-09-11 18:13, dchapyshev(a)svn.reactos.org wrote: > Author: dchapyshev > Date: Sun Sep 11 16:13:46 2016 > New Revision: 72655 > > URL: http://svn.reactos.org/svn/reactos?rev=72655&view=rev > Log: > [NtUser] Try to fix tests for user32:EnumDisplaySettings > > Modified: > trunk/reactos/win32ss/user/ntuser/display.c > > Modified: trunk/reactos/win32ss/user/ntuser/display.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/displa… > ============================================================================== > --- trunk/reactos/win32ss/user/ntuser/display.c [iso-8859-1] (original) > +++ trunk/reactos/win32ss/user/ntuser/display.c [iso-8859-1] Sun Sep 11 16:13:46 2016 > @@ -443,7 +443,7 @@ > { > /* No device found */ > ERR("No PDEV found!\n"); > - return STATUS_UNSUCCESSFUL; > + return STATUS_INVALID_PARAMETER_1; > } > > *ppdm = ppdev->pdmwDev; > @@ -474,7 +474,7 @@ > { > /* No device found */ > ERR("No device found!\n"); > - return STATUS_UNSUCCESSFUL; > + return STATUS_INVALID_PARAMETER_1; > } > > iFoundMode = 0; > @@ -571,13 +571,18 @@ > > _SEH2_TRY > { > - ProbeForWrite(lpDevMode, sizeof(DEVMODEW), 1); > + ProbeForRead(lpDevMode, sizeof(DEVMODEW), 1); > + > + cbSize = lpDevMode->dmSize; > + cbExtra = lpDevMode->dmDriverExtra; > + > + ProbeForWrite(lpDevMode, cbSize + cbExtra, 1); > } > _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) > { > _SEH2_YIELD(return _SEH2_GetExceptionCode()); > } > - _SEH2_END > + _SEH2_END; > > if (lpDevMode->dmSize != sizeof(DEVMODEW)) > { > @@ -586,31 +591,30 @@ > > if (pustrDevice) > { > - if (pustrDevice->Buffer == NULL || pustrDevice->Length == 0) > - { > - Status = STATUS_INVALID_PARAMETER_1; > - } > - > /* Initialize destination string */ > RtlInitEmptyUnicodeString(&ustrDevice, awcDevice, sizeof(awcDevice)); > > _SEH2_TRY > { > /* Probe the UNICODE_STRING and the buffer */ > - ProbeForRead(pustrDevice, sizeof(UNICODE_STRING), 1); > - ProbeForRead(pustrDevice->Buffer, pustrDevice->Length, 1); > + ProbeForReadUnicodeString(pustrDevice); > + > + if (!pustrDevice->Length || !pustrDevice->Buffer) > + ExRaiseStatus(STATUS_NO_MEMORY); > + > + ProbeForRead(pustrDevice->Buffer, pustrDevice->Length, sizeof(UCHAR)); > > /* Copy the string */ > RtlCopyUnicodeString(&ustrDevice, pustrDevice); > } > _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) > { > - _SEH2_YIELD(return _SEH2_GetExceptionCode()); > - } > - _SEH2_END > + _SEH2_YIELD(return STATUS_INVALID_PARAMETER_1); > + } > + _SEH2_END; > > pustrDevice = &ustrDevice; > - } > + } > > /* Acquire global USER lock */ > UserEnterShared(); > @@ -642,11 +646,6 @@ > /* Copy some information back */ > _SEH2_TRY > { > - ProbeForRead(lpDevMode, sizeof(DEVMODEW), 1); > - cbSize = lpDevMode->dmSize; > - cbExtra = lpDevMode->dmDriverExtra; > - > - ProbeForWrite(lpDevMode, cbSize + cbExtra, 1); > /* Output what we got */ > RtlCopyMemory(lpDevMode, pdm, min(cbSize, pdm->dmSize)); > > @@ -663,13 +662,6 @@ > Status = _SEH2_GetExceptionCode(); > } > _SEH2_END; > - } > - else > - { > - if (Status == STATUS_UNSUCCESSFUL) > - { > - Status = STATUS_INVALID_PARAMETER_1; > - } > } > > return Status; > >

1 0

Re: [ros-dev] [ros-diffs] [gedmurphy] 72594: [FLTMGR] - Implement FltLoadFilter and stub FltUnloadFilter - Register contexts if the caller requests (coming later) - Stubpliment FltStartFiltering
by Thomas Faber 07 Sep '16

07 Sep '16

On 2016-09-06 18:47, gedmurphy(a)svn.reactos.org wrote: > @@ -116,12 +150,21 @@ > InitializeListHead(&Filter->PortList.mList); > Filter->PortList.mCount = 0; > > + /* We got this far, assume success from here */ > + Status = STATUS_SUCCESS; > + > /* Check if the caller requested any context data */ > if (Registration->ContextRegistration) > { > - // register the context information > - } > - > + /* Register the contexts for this filter */ > + Status = FltpRegisterContexts(Filter, Registration->ContextRegistration); > + if (NT_SUCCESS(Status)) I think you mean !NT_SUCCESS? > + { > + goto Quit; > + } > + } > + > + /* Check if the caller is registering any callbacks */ > if (Registration->OperationRegistration) > { > /* The callback data comes after the fixed struct */ > @@ -162,7 +205,7 @@ > // - More stuff?? > // > > -//Quit: > +Quit: > if (!NT_SUCCESS(Status)) > { > // Add cleanup for context resources

1 0

Re: [ros-dev] [ros-diffs] [gedmurphy] 72595: [FLTMGR] - Add stubs for context registration - Add dispatch and device control handlers - Implement the load IOCTL. We can now load minifilters with 'fltmc load <myfilter>'
by Thomas Faber 07 Sep '16

07 Sep '16

On 2016-09-06 18:59, gedmurphy(a)svn.reactos.org wrote: > +NTSTATUS > +HandleLoadUnloadIoctl(_In_ PDEVICE_OBJECT DeviceObject, > + _Inout_ PIRP Irp) > +{ > + PIO_STACK_LOCATION StackPtr; > + UNICODE_STRING Name; > + PFILTER_NAME FilterName; > + ULONG BufferLength; > + ULONG ControlCode; > + > + /* Get the IOCTL data from the stack pointer */ > + StackPtr = IoGetCurrentIrpStackLocation(Irp); > + BufferLength = StackPtr->Parameters.DeviceIoControl.InputBufferLength; > + ControlCode = StackPtr->Parameters.DeviceIoControl.IoControlCode; > + > + FLT_ASSERT(ControlCode == IOCTL_LOAD_FILTER || ControlCode == IOCTL_UNLOAD_FILTER); > + > + /* Make sure the buffer is valid */ > + if (BufferLength < sizeof(FILTER_NAME)) > + return STATUS_INVALID_PARAMETER; > + > + /* Convert the file name buffer into a string */ > + FilterName = (PFILTER_NAME)Irp->AssociatedIrp.SystemBuffer; > + Name.Length = FilterName->Length; > + Name.MaximumLength = FilterName->Length; > + Name.Buffer = (PWCH)((PCHAR)FilterName + FIELD_OFFSET(FILTER_NAME, FilterName[0])); You're missing a check for BufferLength < FIELD_OFFSET(FILTER_NAME, FilterName[FilterName->Length / sizeof(WCHAR)]) > + > + /* Forward the request to our Flt routines */ > + if (ControlCode == IOCTL_LOAD_FILTER) > + { > + return FltLoadFilter(&Name); > + } > + else > + { > + return FltUnloadFilter(&Name); > + } > +}

1 0

Re: [ros-dev] where is the final report for AHCI GSoC?
by Alex Ionescu 05 Sep '16

05 Sep '16

Hi Alexander, All the check-ins were mirrored in our SVN repository branch, and the official GSOC page has a link to the Wiki. Best regards, Alex Ionescu On Sun, Sep 4, 2016 at 2:57 AM, Alexander Rechitskiy <art1st-tm(a)yandex.ru> wrote: > Thank you for prompt answer. > Me and some other ppl have worried about getting not so much news about > progress of this project as we expected. I did not know that you have > decided to use github for final report. > > 04.09.2016, 12:47, "Aman Priyadarshi" <aman.eureka(a)gmail.com>: > > Hi, > > Please read the wiki page > https://github.com/amaneureka/reactos/wiki/Storahci > > Link is also available of GSoC website. > ᐧ > > > Regards > *Aman Priyadarshi* > *www.atomixos.com <http://www.atomixos.com/>* > > On Sun, Sep 4, 2016 at 3:07 PM, Alexander Rechitskiy <art1st-tm(a)yandex.ru> > wrote: > > > > 04.09.2016, 12:09, "Alexander Rechitskiy" art1st-tm(a)yandex.ru: > > > > Have we failed this project? > > > > > As I know, project was NOT failed! I just wanted to read the final report. > > -- > Best regards, > > > _______________________________________________ > 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 > > > > -- > Best regards, > > > _______________________________________________ > Ros-dev mailing list > Ros-dev(a)reactos.org > http://www.reactos.org/mailman/listinfo/ros-dev > >

1 0

where is the final report for AHCI GSoC?
by Alexander Rechitskiy 04 Sep '16

04 Sep '16

2 3

Infrastructure team application - Resending after subscribing
by Vyronas Tsingaras 29 Aug '16

29 Aug '16

Dear all, My name is Vyron Tsingaras a 23 year old currently employed as a Systems and Network Administrator at AMD Telecom S.A. My work duties involve maintainance and installation of Linux servers, XenSerevr hypervisors, networking equipment (Cisco ISR, Cisco ITP, Cisco ASA, Fortigate firewalls, Mikrotik routers, pfSense firewalls) and basic MySQL administration. I love using Ops automation tools (pref. Ansible) and have experience scripting and developing in Python. I have followed ReactOS for quite some time and me being an unqualified programmer for systems programming for contributing code to such a complex project I feel that this is a position I am better qualified for. I am from Greece (UTC+2) and my work hours are 9-5.30. I will be available after work hours and on weekends always providing notifcation if I will be unavailable. I feel excited at the prospect of contributing to an open source project of this scale and look forward to working with all of you should I be accepted. Best Regards, Vyron Tsingaras.

1 0

Re: [ros-dev] [ros-diffs] [gadamopoulos] 72398: [SHELL32] - Fail to delete any file if one is invalid. - Patch by andy-123 CORE-9959
by Thomas Faber 28 Aug '16

28 Aug '16

On 2016-08-20 11:43, gadamopoulos(a)svn.reactos.org wrote: > --- trunk/reactos/dll/win32/shell32/shlfileop.cpp [iso-8859-1] (original) > +++ trunk/reactos/dll/win32/shell32/shlfileop.cpp [iso-8859-1] Sat Aug 20 09:43:09 2016 > @@ -1480,6 +1480,19 @@ > return 0; > } > > + /* Check files. Do not delete one if one file does not exists */ > + for (i = 0; i < flFrom->dwNumFiles; i++) > + { > + fileEntry = &flFrom->feFiles[i]; > + > + if ((HANDLE)fileEntry->attributes == INVALID_HANDLE_VALUE) What's wrong with INVALID_FILE_ATTRIBUTES? > + { > + // This is a windows 2003 server specific value which has been removed. > + // Later versions of windows return ERROR_FILE_NOT_FOUND. > + return ERROR_SHELL_INTERNAL_FILE_NOT_FOUND; > + } > + } > + > for (i = 0; i < flFrom->dwNumFiles; i++) > { > bPathExists = TRUE;

1 0

Status Meeting (August 2016)
by Aleksey Bragin 27 Aug '16

27 Aug '16

Hello, Let me invite you to the monthly status meeting taking place this Thursday, 25th of August, 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. The backup place to conduct meetings is #reactos-meeting channel on Freenode. 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

2 1

Re: [ros-dev] [ros-diffs] [jgardou] 72435: [KDGDB] - Implement setting and removing breakpoints - Implement single-stepping - Only get past the breakpoint instruction when it comes from RtlpBreakWithStatus - Implement writi...
by Pierre Schweitzer 23 Aug '16

23 Aug '16

Sorry, I had forgotten this: Le 23/08/2016 à 00:24, jgardou(a)svn.reactos.org a écrit : > Author: jgardou > Date: Mon Aug 22 22:24:30 2016 > New Revision: 72435 > > URL: http://svn.reactos.org/svn/reactos?rev=72435&view=rev > Log: > [KDGDB] > - Implement setting and removing breakpoints > - Implement single-stepping > - Only get past the breakpoint instruction when it comes from RtlpBreakWithStatus > - Implement writing to memory > - Always send the thread raising the exception to GDB to avoid confusing it > - Let GDB find the program counter alone by querying the registers (support was already there before.) > - Properly escape special characters on input > Ladies and gentlemen : the almost-fully functional GDB stub. (still no real multi-process support :-( ) > To enable , in CMakeCache.txt : > - Set GDB:BOOL=TRUE > - Set _WINKD_:BOOL=TRUE > - Set KDBG:BOOL=FALSE > To do : give GDB the list of loaded drivers. Loading ntoskrnl.exe symbols at 0x80801000 already does a good enough job. > Default output is on COM1. Can be configure to any othe COM port with usual kernel options. > Hope you'll like it as much as I do ;-) > > Modified: > trunk/reactos/drivers/base/kdgdb/gdb_input.c > trunk/reactos/drivers/base/kdgdb/gdb_receive.c > trunk/reactos/drivers/base/kdgdb/gdb_send.c > trunk/reactos/drivers/base/kdgdb/i386_sup.c > trunk/reactos/drivers/base/kdgdb/kdcom.c > trunk/reactos/drivers/base/kdgdb/kdgdb.h > trunk/reactos/drivers/base/kdgdb/kdpacket.c > > Modified: trunk/reactos/drivers/base/kdgdb/gdb_input.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/base/kdgdb/gdb_inp… > ============================================================================== > --- trunk/reactos/drivers/base/kdgdb/gdb_input.c [iso-8859-1] (original) > +++ trunk/reactos/drivers/base/kdgdb/gdb_input.c [iso-8859-1] Mon Aug 22 22:24:30 2016 > @@ -9,6 +9,11 @@ > > /* LOCALS *********************************************************************/ > static ULONG_PTR gdb_run_tid; > +static struct > +{ > + ULONG_PTR Address; > + ULONG Handle; > +} BreakPointHandles[32]; > > > /* GLOBALS ********************************************************************/ > @@ -203,16 +208,15 @@ > > if (!Resuming) > { > + /* Report the idle thread */ > +#if MONOPROCESS > + ptr += sprintf(ptr, "1"); > +#else > + ptr += sprintf(gdb, "p1.1"); > +#endif That looks highly suspicious? Shouldn't it be ptr += sprintf(ptr, "p1.1"); -- Pierre Schweitzer <pierre at reactos.org> System & Network Administrator Senior Kernel Developer ReactOS Deutschland e.V.

1 0

Re: [ros-dev] [ros-diffs] [jgardou] 72435: [KDGDB] - Implement setting and removing breakpoints - Implement single-stepping - Only get past the breakpoint instruction when it comes from RtlpBreakWithStatus - Implement writi...
by Pierre Schweitzer 23 Aug '16

23 Aug '16

Hi Jérome, First of all, amazing work. Thanks for it! Is this page still relevant regarding debugging with GDB? https://www.reactos.org/wiki/GDB Or is there a new way to configure ReactOS/client side (Linux, ideally :-p). Cheers, Pierre Le 23/08/2016 à 00:24, jgardou(a)svn.reactos.org a écrit : > Author: jgardou > Date: Mon Aug 22 22:24:30 2016 > New Revision: 72435 > > URL: http://svn.reactos.org/svn/reactos?rev=72435&view=rev > Log: > [KDGDB] > - Implement setting and removing breakpoints > - Implement single-stepping > - Only get past the breakpoint instruction when it comes from RtlpBreakWithStatus > - Implement writing to memory > - Always send the thread raising the exception to GDB to avoid confusing it > - Let GDB find the program counter alone by querying the registers (support was already there before.) > - Properly escape special characters on input > Ladies and gentlemen : the almost-fully functional GDB stub. (still no real multi-process support :-( ) > To enable , in CMakeCache.txt : > - Set GDB:BOOL=TRUE > - Set _WINKD_:BOOL=TRUE > - Set KDBG:BOOL=FALSE > To do : give GDB the list of loaded drivers. Loading ntoskrnl.exe symbols at 0x80801000 already does a good enough job. > Default output is on COM1. Can be configure to any othe COM port with usual kernel options. > Hope you'll like it as much as I do ;-) > > Modified: > trunk/reactos/drivers/base/kdgdb/gdb_input.c > trunk/reactos/drivers/base/kdgdb/gdb_receive.c > trunk/reactos/drivers/base/kdgdb/gdb_send.c > trunk/reactos/drivers/base/kdgdb/i386_sup.c > trunk/reactos/drivers/base/kdgdb/kdcom.c > trunk/reactos/drivers/base/kdgdb/kdgdb.h > trunk/reactos/drivers/base/kdgdb/kdpacket.c > > Modified: trunk/reactos/drivers/base/kdgdb/gdb_input.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/base/kdgdb/gdb_inp… > ============================================================================== > --- trunk/reactos/drivers/base/kdgdb/gdb_input.c [iso-8859-1] (original) > +++ trunk/reactos/drivers/base/kdgdb/gdb_input.c [iso-8859-1] Mon Aug 22 22:24:30 2016 > @@ -9,6 +9,11 @@ > > /* LOCALS *********************************************************************/ > static ULONG_PTR gdb_run_tid; > +static struct > +{ > + ULONG_PTR Address; > + ULONG Handle; > +} BreakPointHandles[32]; > > > /* GLOBALS ********************************************************************/ > @@ -203,16 +208,15 @@ > > if (!Resuming) > { > + /* Report the idle thread */ > +#if MONOPROCESS > + ptr += sprintf(ptr, "1"); > +#else > + ptr += sprintf(gdb, "p1.1"); > +#endif > /* Initialize the entries */ > CurrentProcessEntry = ProcessListHead->Flink; > CurrentThreadEntry = NULL; > - > - /* Start with idle thread */ > -#if MONOPROCESS > - ptr = gdb_out + sprintf(gdb_out, "m1"); > -#else > - ptr = gdb_out + sprintf(gdb_out, "mp1.1"); > -#endif > FirstThread = FALSE; > } > > @@ -306,8 +310,8 @@ > Thread = find_thread(Pid, Tid); > Process = CONTAINING_RECORD(Thread->Tcb.Process, EPROCESS, Pcb); > #else > - Pid = hex_to_pid(&gdb_input[2]); > - Tid = hex_to_tid(strstr(gdb_input, ".") + 1); > + Pid = hex_to_pid(&gdb_input[18]); > + Tid = hex_to_tid(strstr(&gdb_input[18], ".") + 1); > > /* We cannot use PsLookupProcessThreadByCid as we could be running at any IRQL. > * So loop. */ > @@ -394,21 +398,23 @@ > KDDBGPRINT("Wrong API number (%lu) after DbgKdReadVirtualMemoryApi request.\n", State->ApiNumber); > } > > - /* Check status */ > - if (!NT_SUCCESS(State->ReturnStatus)) > + /* Check status. Allow to send partial data. */ > + if (!MessageData->Length && !NT_SUCCESS(State->ReturnStatus)) > send_gdb_ntstatus(State->ReturnStatus); > else > send_gdb_memory(MessageData->Buffer, MessageData->Length); > KdpSendPacketHandler = NULL; > KdpManipulateStateHandler = NULL; > > -#if !MONOPROCESS > +#if MONOPROCESS > + if (gdb_dbg_tid != 0) > /* Reset the TLB */ > +#else > if ((gdb_dbg_pid != 0) && gdb_pid_to_handle(gdb_dbg_pid) != PsGetCurrentProcessId()) > +#endif > { > __writecr3(PsGetCurrentProcess()->Pcb.DirectoryTableBase[0]); > } > -#endif > } > > static > @@ -427,8 +433,29 @@ > MessageData->Length = 0; > *MessageLength = 0; > > -#if !MONOPROCESS > /* Set the TLB according to the process being read. Pid 0 means any process. */ > +#if MONOPROCESS > + if ((gdb_dbg_tid != 0) && gdb_tid_to_handle(gdb_dbg_tid) != PsGetCurrentThreadId()) > + { > + PETHREAD AttachedThread = find_thread(0, gdb_dbg_tid); > + PKPROCESS AttachedProcess; > + if (AttachedThread == NULL) > + { > + KDDBGPRINT("The current GDB debug thread is invalid!"); > + send_gdb_packet("E03"); > + return (KDSTATUS)-1; > + } > + > + AttachedProcess = AttachedThread->Tcb.Process; > + if (AttachedProcess == NULL) > + { > + KDDBGPRINT("The current GDB debug thread is invalid!"); > + send_gdb_packet("E03"); > + return (KDSTATUS)-1; > + } > + __writecr3(AttachedProcess->DirectoryTableBase[0]); > + } > +#else > if ((gdb_dbg_pid != 0) && gdb_pid_to_handle(gdb_dbg_pid) != PsGetCurrentProcessId()) > { > PEPROCESS AttachedProcess = find_process(gdb_dbg_pid); > @@ -436,7 +463,7 @@ > { > KDDBGPRINT("The current GDB debug thread is invalid!"); > send_gdb_packet("E03"); > - return gdb_receive_and_interpret_packet(State, MessageData, MessageLength, KdContext); > + return (KDSTATUS)-1; > } > __writecr3(AttachedProcess->Pcb.DirectoryTableBase[0]); > } > @@ -448,6 +475,388 @@ > /* KD will reply with KdSendPacket. Catch it */ > KdpSendPacketHandler = ReadMemorySendHandler; > > + return KdPacketReceived; > +} > + > +static > +void > +WriteMemorySendHandler( > + _In_ ULONG PacketType, > + _In_ PSTRING MessageHeader, > + _In_ PSTRING MessageData) > +{ > + DBGKD_MANIPULATE_STATE64* State = (DBGKD_MANIPULATE_STATE64*)MessageHeader->Buffer; > + > + if (PacketType != PACKET_TYPE_KD_STATE_MANIPULATE) > + { > + // KdAssert > + KDDBGPRINT("Wrong packet type (%lu) received after DbgKdWriteVirtualMemoryApi request.\n", PacketType); > + while (1); > + } > + > + if (State->ApiNumber != DbgKdWriteVirtualMemoryApi) > + { > + KDDBGPRINT("Wrong API number (%lu) after DbgKdWriteVirtualMemoryApi request.\n", State->ApiNumber); > + } > + > + /* Check status */ > + if (!NT_SUCCESS(State->ReturnStatus)) > + send_gdb_ntstatus(State->ReturnStatus); > + else > + send_gdb_packet("OK"); > + KdpSendPacketHandler = NULL; > + KdpManipulateStateHandler = NULL; > + > +#if MONOPROCESS > + if (gdb_dbg_tid != 0) > + /* Reset the TLB */ > +#else > + if ((gdb_dbg_pid != 0) && gdb_pid_to_handle(gdb_dbg_pid) != PsGetCurrentProcessId()) > +#endif > + { > + __writecr3(PsGetCurrentProcess()->Pcb.DirectoryTableBase[0]); > + } > +} > + > +static > +KDSTATUS > +handle_gdb_write_mem( > + _Out_ DBGKD_MANIPULATE_STATE64* State, > + _Out_ PSTRING MessageData, > + _Out_ PULONG MessageLength, > + _Inout_ PKD_CONTEXT KdContext) > +{ > + /* Maximal input buffer is 0x1000. Each byte is encoded on two bytes by GDB */ > + static UCHAR OutBuffer[0x800]; > + ULONG BufferLength; > + char* blob_ptr; > + UCHAR* OutPtr; > + > + State->ApiNumber = DbgKdWriteVirtualMemoryApi; > + State->ReturnStatus = STATUS_SUCCESS; /* ? */ > + State->Processor = CurrentStateChange.Processor; > + State->ProcessorLevel = CurrentStateChange.ProcessorLevel; > + > + /* Set the TLB according to the process being read. Pid 0 means any process. */ > +#if MONOPROCESS > + if ((gdb_dbg_tid != 0) && gdb_tid_to_handle(gdb_dbg_tid) != PsGetCurrentThreadId()) > + { > + PETHREAD AttachedThread = find_thread(0, gdb_dbg_tid); > + PKPROCESS AttachedProcess; > + if (AttachedThread == NULL) > + { > + KDDBGPRINT("The current GDB debug thread is invalid!"); > + send_gdb_packet("E03"); > + return (KDSTATUS)-1; > + } > + > + AttachedProcess = AttachedThread->Tcb.Process; > + if (AttachedProcess == NULL) > + { > + KDDBGPRINT("The current GDB debug thread is invalid!"); > + send_gdb_packet("E03"); > + return (KDSTATUS)-1; > + } > + __writecr3(AttachedProcess->DirectoryTableBase[0]); > + } > +#else > + if ((gdb_dbg_pid != 0) && gdb_pid_to_handle(gdb_dbg_pid) != PsGetCurrentProcessId()) > + { > + PEPROCESS AttachedProcess = find_process(gdb_dbg_pid); > + if (AttachedProcess == NULL) > + { > + KDDBGPRINT("The current GDB debug thread is invalid!"); > + send_gdb_packet("E03"); > + return (KDSTATUS)-1; > + } > + __writecr3(AttachedProcess->Pcb.DirectoryTableBase[0]); > + } > +#endif > + > + State->u.WriteMemory.TargetBaseAddress = hex_to_address(&gdb_input[1]); > + BufferLength = hex_to_address(strstr(&gdb_input[1], ",") + 1); > + if (BufferLength == 0) > + { > + /* Nothing to do */ > + send_gdb_packet("OK"); > + return (KDSTATUS)-1; > + } > + > + State->u.WriteMemory.TransferCount = BufferLength; > + MessageData->Length = BufferLength; > + MessageData->Buffer = (CHAR*)OutBuffer; > + > + OutPtr = OutBuffer; > + blob_ptr = strstr(strstr(&gdb_input[1], ",") + 1, ":") + 1; > + while (BufferLength) > + { > + if (BufferLength >= 4) > + { > + *((ULONG*)OutPtr) = *((ULONG*)blob_ptr); > + OutPtr += 4; > + blob_ptr += 4; > + BufferLength -= 4; > + } > + else if (BufferLength >= 2) > + { > + *((USHORT*)OutPtr) = *((USHORT*)blob_ptr); > + OutPtr += 2; > + blob_ptr += 2; > + BufferLength -= 2; > + } > + else > + { > + *OutPtr++ = *blob_ptr++; > + BufferLength--; > + } > + } > + > + /* KD will reply with KdSendPacket. Catch it */ > + KdpSendPacketHandler = WriteMemorySendHandler; > + > + return KdPacketReceived; > +} > + > +static > +void > +WriteBreakPointSendHandler( > + _In_ ULONG PacketType, > + _In_ PSTRING MessageHeader, > + _In_ PSTRING MessageData) > +{ > + DBGKD_MANIPULATE_STATE64* State = (DBGKD_MANIPULATE_STATE64*)MessageHeader->Buffer; > + > + if (PacketType != PACKET_TYPE_KD_STATE_MANIPULATE) > + { > + // KdAssert > + KDDBGPRINT("Wrong packet type (%lu) received after DbgKdWriteBreakPointApi request.\n", PacketType); > + while (1); > + } > + > + if (State->ApiNumber != DbgKdWriteBreakPointApi) > + { > + KDDBGPRINT("Wrong API number (%lu) after DbgKdWriteBreakPointApi request.\n", State->ApiNumber); > + } > + > + /* Check status */ > + if (!NT_SUCCESS(State->ReturnStatus)) > + { > + KDDBGPRINT("Inserting breakpoint failed!\n"); > + send_gdb_ntstatus(State->ReturnStatus); > + } > + else > + { > + /* Keep track of the address+handle couple */ > + ULONG i; > + for (i = 0; i < (sizeof(BreakPointHandles) / sizeof(BreakPointHandles[0])); i++) > + { > + if (BreakPointHandles[i].Address == 0) > + { > + BreakPointHandles[i].Address = (ULONG_PTR)State->u.WriteBreakPoint.BreakPointAddress; > + BreakPointHandles[i].Handle = State->u.WriteBreakPoint.BreakPointHandle; > + break; > + } > + } > + send_gdb_packet("OK"); > + } > + KdpSendPacketHandler = NULL; > + KdpManipulateStateHandler = NULL; > +} > + > +static > +KDSTATUS > +handle_gdb_insert_breakpoint( > + _Out_ DBGKD_MANIPULATE_STATE64* State, > + _Out_ PSTRING MessageData, > + _Out_ PULONG MessageLength, > + _Inout_ PKD_CONTEXT KdContext) > +{ > + State->ReturnStatus = STATUS_SUCCESS; /* ? */ > + State->Processor = CurrentStateChange.Processor; > + State->ProcessorLevel = CurrentStateChange.ProcessorLevel; > + if (MessageData) > + MessageData->Length = 0; > + *MessageLength = 0; > + > + switch (gdb_input[1]) > + { > + case '0': > + { > + ULONG_PTR Address = hex_to_address(&gdb_input[3]); > + ULONG i; > + BOOLEAN HasFreeSlot = FALSE; > + > + KDDBGPRINT("Inserting breakpoint at %p.\n", (void*)Address); > + > + for (i = 0; i < (sizeof(BreakPointHandles) / sizeof(BreakPointHandles[0])); i++) > + { > + if (BreakPointHandles[i].Address == 0) > + HasFreeSlot = TRUE; > + } > + > + if (!HasFreeSlot) > + { > + /* We don't have a way to keep track of this break point. Fail. */ > + KDDBGPRINT("No breakpoint slot available!\n"); > + send_gdb_packet("E01"); > + return (KDSTATUS)-1; > + } > + > + State->ApiNumber = DbgKdWriteBreakPointApi; > + State->u.WriteBreakPoint.BreakPointAddress = Address; > + /* FIXME : ignoring all other Z0 arguments */ > + > + /* KD will reply with KdSendPacket. Catch it */ > + KdpSendPacketHandler = WriteBreakPointSendHandler; > + return KdPacketReceived; > + } > + } > + > + KDDBGPRINT("Unhandled 'Z' packet: %s\n", gdb_input); > + send_gdb_packet("E01"); > + return (KDSTATUS)-1; > +} > + > +static > +void > +RestoreBreakPointSendHandler( > + _In_ ULONG PacketType, > + _In_ PSTRING MessageHeader, > + _In_ PSTRING MessageData) > +{ > + DBGKD_MANIPULATE_STATE64* State = (DBGKD_MANIPULATE_STATE64*)MessageHeader->Buffer; > + ULONG i; > + > + if (PacketType != PACKET_TYPE_KD_STATE_MANIPULATE) > + { > + // KdAssert > + KDDBGPRINT("Wrong packet type (%lu) received after DbgKdRestoreBreakPointApi request.\n", PacketType); > + while (1); > + } > + > + if (State->ApiNumber != DbgKdRestoreBreakPointApi) > + { > + KDDBGPRINT("Wrong API number (%lu) after DbgKdRestoreBreakPointApi request.\n", State->ApiNumber); > + } > + > + /* We ignore failure here. If DbgKdRestoreBreakPointApi fails, > + * this means that the breakpoint was already invalid for KD. So clean it up on our side. */ > + for (i = 0; i < (sizeof(BreakPointHandles) / sizeof(BreakPointHandles[0])); i++) > + { > + if (BreakPointHandles[i].Handle == State->u.RestoreBreakPoint.BreakPointHandle) > + { > + BreakPointHandles[i].Address = 0; > + BreakPointHandles[i].Handle = 0; > + break; > + } > + } > + > + send_gdb_packet("OK"); > + > + KdpSendPacketHandler = NULL; > + KdpManipulateStateHandler = NULL; > +} > + > +static > +KDSTATUS > +handle_gdb_remove_breakpoint( > + _Out_ DBGKD_MANIPULATE_STATE64* State, > + _Out_ PSTRING MessageData, > + _Out_ PULONG MessageLength, > + _Inout_ PKD_CONTEXT KdContext) > +{ > + State->ReturnStatus = STATUS_SUCCESS; /* ? */ > + State->Processor = CurrentStateChange.Processor; > + State->ProcessorLevel = CurrentStateChange.ProcessorLevel; > + if (MessageData) > + MessageData->Length = 0; > + *MessageLength = 0; > + > + switch (gdb_input[1]) > + { > + case '0': > + { > + ULONG_PTR Address = hex_to_address(&gdb_input[3]); > + ULONG i, Handle = 0; > + > + KDDBGPRINT("Removing breakpoint on %p.\n", (void*)Address); > + > + for (i = 0; i < (sizeof(BreakPointHandles) / sizeof(BreakPointHandles[0])); i++) > + { > + if (BreakPointHandles[i].Address == Address) > + { > + Handle = BreakPointHandles[i].Handle; > + break; > + } > + } > + > + if (Handle == 0) > + { > + KDDBGPRINT("Received %s, but breakpoint was never inserted ?!\n", gdb_input); > + send_gdb_packet("E01"); > + return (KDSTATUS)-1; > + } > + > + State->ApiNumber = DbgKdRestoreBreakPointApi; > + State->u.RestoreBreakPoint.BreakPointHandle = Handle; > + /* FIXME : ignoring all other z0 arguments */ > + > + /* KD will reply with KdSendPacket. Catch it */ > + KdpSendPacketHandler = RestoreBreakPointSendHandler; > + return KdPacketReceived; > + } > + } > + > + KDDBGPRINT("Unhandled 'Z' packet: %s\n", gdb_input); > + send_gdb_packet("E01"); > + return (KDSTATUS)-1; > +} > + > +static > +KDSTATUS > +handle_gdb_c( > + _Out_ DBGKD_MANIPULATE_STATE64* State, > + _Out_ PSTRING MessageData, > + _Out_ PULONG MessageLength, > + _Inout_ PKD_CONTEXT KdContext) > +{ > + /* Tell GDB everything is fine, we will handle it */ > + send_gdb_packet("OK"); > + > + if (CurrentStateChange.NewState == DbgKdExceptionStateChange) > + { > + DBGKM_EXCEPTION64* Exception = &CurrentStateChange.u.Exception; > + ULONG_PTR ProgramCounter = KdpGetContextPc(&CurrentContext); > + > + /* See if we should update the program counter */ > + if (Exception && (Exception->ExceptionRecord.ExceptionCode == STATUS_BREAKPOINT) > + && ProgramCounter == KdDebuggerDataBlock->BreakpointWithStatus.Pointer) > + { > + /* We must get past the breakpoint instruction */ > + KdpSetContextPc(&CurrentContext, ProgramCounter + KD_BREAKPOINT_SIZE); > + > + SetContextManipulateHandler(State, MessageData, MessageLength, KdContext); > + KdpManipulateStateHandler = ContinueManipulateStateHandler; > + return KdPacketReceived; > + } > + } > + > + return ContinueManipulateStateHandler(State, MessageData, MessageLength, KdContext); > +} > + > +static > +KDSTATUS > +handle_gdb_s( > + _Out_ DBGKD_MANIPULATE_STATE64* State, > + _Out_ PSTRING MessageData, > + _Out_ PULONG MessageLength, > + _Inout_ PKD_CONTEXT KdContext) > +{ > + KDDBGPRINT("Single stepping.\n"); > + /* Set CPU single step mode and continue */ > + KdpSetSingleStep(&CurrentContext); > + SetContextManipulateHandler(State, MessageData, MessageLength, KdContext); > + KdpManipulateStateHandler = ContinueManipulateStateHandler; > return KdPacketReceived; > } > > @@ -464,36 +873,19 @@ > if (gdb_input[5] == '?') > { > /* Report what we support */ > - send_gdb_packet("vCont;c;C;s;S"); > + send_gdb_packet("vCont;c;s"); > return (KDSTATUS)-1; > } > > if (strncmp(gdb_input, "vCont;c", 7) == 0) > { > - DBGKM_EXCEPTION64* Exception = NULL; > - > - /* Tell GDB everything is fine, we will handle it */ > - send_gdb_packet("OK"); > - > - if (CurrentStateChange.NewState == DbgKdExceptionStateChange) > - Exception = &CurrentStateChange.u.Exception; > - > - /* See if we should update the program counter (unlike windbg, gdb doesn't do it for us) */ > - if (Exception && (Exception->ExceptionRecord.ExceptionCode == STATUS_BREAKPOINT) > - && (Exception->ExceptionRecord.ExceptionInformation[0] == 0)) > - { > - ULONG_PTR ProgramCounter; > - > - /* So we must get past the breakpoint instruction */ > - ProgramCounter = KdpGetContextPc(&CurrentContext); > - KdpSetContextPc(&CurrentContext, ProgramCounter + KD_BREAKPOINT_SIZE); > - > - SetContextManipulateHandler(State, MessageData, MessageLength, KdContext); > - KdpManipulateStateHandler = ContinueManipulateStateHandler; > - return KdPacketReceived; > - } > - > - return ContinueManipulateStateHandler(State, MessageData, MessageLength, KdContext); > + return handle_gdb_c(State, MessageData, MessageLength, KdContext); > + } > + > + if (strncmp(gdb_input, "vCont;s", 7) == 0) > + { > + > + return handle_gdb_s(State, MessageData, MessageLength, KdContext); > } > } > > @@ -524,11 +916,14 @@ > { > case '?': > /* Send the Status */ > - gdb_send_exception(TRUE); > + gdb_send_exception(); > break; > case '!': > send_gdb_packet("OK"); > break; > + case 'c': > + Status = handle_gdb_c(State, MessageData, MessageLength, KdContext); > + break; > case 'g': > gdb_send_registers(); > break; > @@ -544,17 +939,28 @@ > case 'q': > handle_gdb_query(); > break; > + case 's': > + Status = handle_gdb_s(State, MessageData, MessageLength, KdContext); > + break; > case 'T': > handle_gdb_thread_alive(); > break; > case 'v': > Status = handle_gdb_v(State, MessageData, MessageLength, KdContext); > break; > + case 'X': > + Status = handle_gdb_write_mem(State, MessageData, MessageLength, KdContext); > + break; > + case 'z': > + Status = handle_gdb_remove_breakpoint(State, MessageData, MessageLength, KdContext); > + break; > + case 'Z': > + Status = handle_gdb_insert_breakpoint(State, MessageData, MessageLength, KdContext); > + break; > default: > - /* We don't know how to handle this request. Maybe this is something for KD */ > - State->ReturnStatus = STATUS_NOT_SUPPORTED; > + /* We don't know how to handle this request. */ > KDDBGPRINT("Unsupported GDB command: %s.\n", gdb_input); > - return KdPacketReceived; > + send_gdb_packet(""); > } > } while (Status == (KDSTATUS)-1); > > > Modified: trunk/reactos/drivers/base/kdgdb/gdb_receive.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/base/kdgdb/gdb_rec… > ============================================================================== > --- trunk/reactos/drivers/base/kdgdb/gdb_receive.c [iso-8859-1] (original) > +++ trunk/reactos/drivers/base/kdgdb/gdb_receive.c [iso-8859-1] Mon Aug 22 22:24:30 2016 > @@ -30,7 +30,7 @@ > NTAPI > gdb_receive_packet(_Inout_ PKD_CONTEXT KdContext) > { > - char* ByteBuffer = gdb_input; > + UCHAR* ByteBuffer = (UCHAR*)gdb_input; > UCHAR Byte; > KDSTATUS Status; > CHAR CheckSum = 0, ReceivedCheckSum; > @@ -60,9 +60,18 @@ > *ByteBuffer = '\0'; > break; > } > - > + CheckSum += (CHAR)Byte; > + > + /* See if we should escape */ > + if (Byte == 0x7d) > + { > + Status = KdpReceiveByte(&Byte); > + if (Status != KdPacketReceived) > + return Status; > + CheckSum += (CHAR)Byte; > + Byte ^= 0x20; > + } > *ByteBuffer++ = Byte; > - CheckSum += (CHAR)Byte; > } > > /* Get Check sum (two bytes) */ > @@ -80,6 +89,7 @@ > if (ReceivedCheckSum != CheckSum) > { > /* Do not acknowledge to GDB */ > + KDDBGPRINT("Check sums don't match!"); > KdpSendByte('-'); > return KdPacketNeedsResend; > } > > Modified: trunk/reactos/drivers/base/kdgdb/gdb_send.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/base/kdgdb/gdb_sen… > ============================================================================== > --- trunk/reactos/drivers/base/kdgdb/gdb_send.c [iso-8859-1] (original) > +++ trunk/reactos/drivers/base/kdgdb/gdb_send.c [iso-8859-1] Mon Aug 22 22:24:30 2016 > @@ -168,7 +168,7 @@ > } > > void > -gdb_send_exception(BOOLEAN WithThread) > +gdb_send_exception() > { > char gdb_out[1024]; > char* ptr = gdb_out; > @@ -184,20 +184,16 @@ > } > else > ptr += sprintf(ptr, "05"); > - if (WithThread) > - { > + > #if MONOPROCESS > - ptr += sprintf(ptr, "thread:%" PRIxPTR ";", > - handle_to_gdb_tid(PsGetThreadId(Thread))); > + ptr += sprintf(ptr, "thread:%" PRIxPTR ";", > + handle_to_gdb_tid(PsGetThreadId(Thread))); > #else > - ptr += sprintf(ptr, "thread:p%" PRIxPTR ".%" PRIxPTR ";", > - handle_to_gdb_pid(PsGetThreadProcessId(Thread)), > - handle_to_gdb_tid(PsGetThreadId(Thread))); > + ptr += sprintf(ptr, "thread:p%" PRIxPTR ".%" PRIxPTR ";", > + handle_to_gdb_pid(PsGetThreadProcessId(Thread)), > + handle_to_gdb_tid(PsGetThreadId(Thread))); > #endif > - } > ptr += sprintf(ptr, "core:%x;", CurrentStateChange.Processor); > - /* Add program counter */ > - gdb_append_pc_to_exception(Thread, ptr); > send_gdb_packet(gdb_out); > } > > > Modified: trunk/reactos/drivers/base/kdgdb/i386_sup.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/base/kdgdb/i386_su… > ============================================================================== > --- trunk/reactos/drivers/base/kdgdb/i386_sup.c [iso-8859-1] (original) > +++ trunk/reactos/drivers/base/kdgdb/i386_sup.c [iso-8859-1] Mon Aug 22 22:24:30 2016 > @@ -251,28 +251,4 @@ > } > } > > -char* > -gdb_append_pc_to_exception( > - _In_ PETHREAD Thread, > - _Inout_ char* ptr) > -{ > - /* Get EIP */ > - unsigned short ptrSize; > - unsigned char* EipPtr = thread_to_reg(Thread, EIP, &ptrSize); > - > - /* Print it */ > - ptr += sprintf(ptr, "08:"); > - *ptr++ = hex_chars[EipPtr[0] >> 4]; > - *ptr++ = hex_chars[EipPtr[0] & 0xF]; > - *ptr++ = hex_chars[EipPtr[1] >> 4]; > - *ptr++ = hex_chars[EipPtr[1] & 0xF]; > - *ptr++ = hex_chars[EipPtr[2] >> 4]; > - *ptr++ = hex_chars[EipPtr[2] & 0xF]; > - *ptr++ = hex_chars[EipPtr[3] >> 4]; > - *ptr++ = hex_chars[EipPtr[3] & 0xF]; > - *ptr++ = ';'; > - *ptr++ = '\0'; > - > - return ptr; > -} > - > + > > Modified: trunk/reactos/drivers/base/kdgdb/kdcom.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/base/kdgdb/kdcom.c… > ============================================================================== > --- trunk/reactos/drivers/base/kdgdb/kdcom.c [iso-8859-1] (original) > +++ trunk/reactos/drivers/base/kdgdb/kdcom.c [iso-8859-1] Mon Aug 22 22:24:30 2016 > @@ -303,7 +303,6 @@ > return KdPacketReceived; > } > > - KDDBGPRINT("CpGetByte returned %u.\n", CpStatus); > return KdPacketTimedOut; > } > > > Modified: trunk/reactos/drivers/base/kdgdb/kdgdb.h > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/base/kdgdb/kdgdb.h… > ============================================================================== > --- trunk/reactos/drivers/base/kdgdb/kdgdb.h [iso-8859-1] (original) > +++ trunk/reactos/drivers/base/kdgdb/kdgdb.h [iso-8859-1] Mon Aug 22 22:24:30 2016 > @@ -19,7 +19,7 @@ > > #include <pstypes.h> > > -#define KDDEBUG /* uncomment to enable debugging this dll */ > +//#define KDDEBUG /* uncomment to enable debugging this dll */ > > /* To undefine once https://sourceware.org/bugzilla/show_bug.cgi?id=17397 is resolved */ > #define MONOPROCESS 1 > @@ -83,7 +83,7 @@ > void send_gdb_packet(_In_ CHAR* Buffer); > void send_gdb_memory(_In_ VOID* Buffer, size_t Length); > void gdb_send_debug_io(_In_ PSTRING String, _In_ BOOLEAN WithPrefix); > -void gdb_send_exception(BOOLEAN WithThread); > +void gdb_send_exception(void); > void send_gdb_ntstatus(_In_ NTSTATUS Status); > extern const char hex_chars[]; > > @@ -113,15 +113,18 @@ > /* arch_sup.c */ > extern void gdb_send_register(void); > extern void gdb_send_registers(void); > -extern char* gdb_append_pc_to_exception(_In_ PETHREAD Thread, _Inout_ char* ptr); > > /* Architecture specific defines. See ntoskrnl/include/internal/arch/ke.h */ > #ifdef _M_IX86 > +/* Handling passing over the breakpoint instruction */ > # define KdpGetContextPc(Context) \ > ((Context)->Eip) > # define KdpSetContextPc(Context, ProgramCounter) \ > ((Context)->Eip = (ProgramCounter)) > # define KD_BREAKPOINT_SIZE sizeof(UCHAR) > +/* Single step mode */ > +# define KdpSetSingleStep(Context) \ > + ((Context)->EFlags |= EFLAGS_TF) > #else > # error "Please define relevant macros for your architecture" > #endif > > Modified: trunk/reactos/drivers/base/kdgdb/kdpacket.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/base/kdgdb/kdpacke… > ============================================================================== > --- trunk/reactos/drivers/base/kdgdb/kdpacket.c [iso-8859-1] (original) > +++ trunk/reactos/drivers/base/kdgdb/kdpacket.c [iso-8859-1] Mon Aug 22 22:24:30 2016 > @@ -160,7 +160,7 @@ > #else > gdb_dbg_pid = handle_to_gdb_pid(PsGetThreadProcessId(Thread)); > #endif > - gdb_send_exception(FALSE); > + gdb_send_exception(); > /* Next receive call will ask for the context */ > KdpManipulateStateHandler = GetContextManipulateHandler; > break; > @@ -394,6 +394,13 @@ > IN PSTRING MessageData, > IN OUT PKD_CONTEXT KdContext) > { > + /* Override if we have some debug print from KD. */ > + if (PacketType == PACKET_TYPE_KD_DEBUG_IO) > + { > + send_kd_debug_io((DBGKD_DEBUG_IO*)MessageHeader->Buffer, MessageData); > + return; > + } > + > /* Maybe we are in a send <-> receive loop that GDB doesn't need to know about */ > if (KdpSendPacketHandler) > { > > -- Pierre Schweitzer <pierre at reactos.org> System & Network Administrator Senior Kernel Developer ReactOS Deutschland e.V.

1 0

Re: [ros-dev] [ros-diffs] [vmikayelyan] 72388: usb: HUB: Add PnP state tracking Added PDO/FDO PnP state tracking, which is done according MSDN's "State Transitions for PnP Devices" topic.
by Hervé Poussineau 20 Aug '16

20 Aug '16

Hi, Le 19/08/2016 à 18:46, vmikayelyan(a)svn.reactos.org a écrit : > Author: vmikayelyan > Date: Fri Aug 19 16:46:18 2016 > New Revision: 72388 > > URL: http://svn.reactos.org/svn/reactos?rev=72388&view=rev > Log: > usb: HUB: Add PnP state tracking > > Added PDO/FDO PnP state tracking, which is done according MSDN's > "State Transitions for PnP Devices" topic. > [...] > +#define INITIALIZE_PNP_STATE(Data) \ > +(Data).PnPState = NotStarted;\ > +(Data).PreviousPnPState = NotStarted; > + > +#define SET_NEW_PNP_STATE(Data, state) \ > +(Data).PnPState = (Data).PnPState;\ You probably meant PreviousPnPState here... > +(Data).PnPState = (state); > + > +#define RESTORE_PREVIOUS_PNP_STATE(Data) \ > +(Data).PnPState = (Data).PreviousPnPState; > + Hervé

2 1

Re: [ros-dev] [ros-diffs] [gedmurphy] 72178: [FLTLIB] - Stub out fltlib.dll. - Add basic implementations for FilterLoad and FilterUnload - Remove the wine code, this lib talks directly to fltmgr.sys and is therefore a react...
by Thomas Faber 11 Aug '16

11 Aug '16

On 2016-08-10 00:10, gedmurphy(a)svn.reactos.org wrote: > + /* Calc and allocate a buffer to hold our filter name */ > + BufferLength = wcslen(lpFilterName) * sizeof(WCHAR); > + FilterName = RtlAllocateHeap(GetProcessHeap(), > + 0, > + BufferLength + sizeof(UNICODE_STRING)); > + if (FilterName == NULL) > + { > + CloseHandle(hFltMgr); > + return HRESULT_FROM_WIN32(ERROR_OUTOFMEMORY); > + } > + > + /* Build up the filter name into a real life string */ > + FilterName->Buffer = (PWCH)(FilterName + 1); > + FilterName->Length = BufferLength; > + FilterName->MaximumLength = BufferLength; > + RtlCopyMemory(FilterName->Buffer, lpFilterName, BufferLength); > + > + /* Tell the filter manager to load the filter for us */ > + dwError = SendIoctl(hFltMgr, > + Load ? IOCTL_LOAD_FILTER : IOCTL_UNLOAD_FILTER, > + FilterName, > + BufferLength + sizeof(UNICODE_STRING)); > + > + /* Cleaup and bail*/ > + CloseHandle(hFltMgr); > + return HRESULT_FROM_WIN32(dwError); > } You're leaking this allocation. > +// Hack - our SDK reports NTDDI_VERSION as 0x05020100 (from _WIN32_WINNT 0x502) > +// which doesn't pass the FLT_MGR_BASELINE check in fltkernel.h > +#define NTDDI_VERSION NTDDI_WS03SP1 We typically set this kind of thing in CMakeLists.txt.. however maybe we should define it this way by default

2 1

Status Meeting (June 2016)
by Aleksey Bragin 26 Jul '16

26 Jul '16

Hello, Let me invite you to the monthly status meeting taking place this Thursday, 28th of June, 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. The backup place to conduct meetings is #reactos-meeting channel on Freenode. 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

1 0

Re: [ros-dev] [ros-diffs] [tthompson] 71945: [NTFS] *AddRun() - Don't leak RunBuffer when encountering errors. Handle exception from FsRtlAddLargeMcbEntry().
by Pierre Schweitzer 24 Jul '16

24 Jul '16

Comment inline Le 15/07/2016 à 17:27, tthompson(a)svn.reactos.org a écrit : > Author: tthompson > Date: Fri Jul 15 15:27:04 2016 > New Revision: 71945 > > URL: http://svn.reactos.org/svn/reactos?rev=71945&view=rev > Log: > [NTFS] > *AddRun() - Don't leak RunBuffer when encountering errors. > Handle exception from FsRtlAddLargeMcbEntry(). > > Modified: > branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c > > Modified: branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c > URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/NTFS/drivers/filesyst… > ============================================================================== > --- branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c [iso-8859-1] (original) > +++ branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c [iso-8859-1] Fri Jul 15 15:27:04 2016 > @@ -88,25 +88,40 @@ > ULONGLONG NextVBN = AttrContext->Record.NonResident.LowestVCN; > > // Allocate some memory for the RunBuffer > - PUCHAR RunBuffer = ExAllocatePoolWithTag(NonPagedPool, Vcb->NtfsInfo.BytesPerFileRecord, TAG_NTFS); > + PUCHAR RunBuffer; > int RunBufferOffset = 0; > > if (!AttrContext->Record.IsNonResident) > return STATUS_INVALID_PARAMETER; > + > + RunBuffer = ExAllocatePoolWithTag(NonPagedPool, Vcb->NtfsInfo.BytesPerFileRecord, TAG_NTFS); > > // Convert the data runs to a map control block > Status = ConvertDataRunsToLargeMCB(DataRun, &DataRunsMCB, &NextVBN); > if (!NT_SUCCESS(Status)) > { > DPRINT1("Unable to convert data runs to MCB (probably ran out of memory)!\n"); > + ExFreePoolWithTag(RunBuffer, TAG_NTFS); > return Status; > } > > // Add newly-assigned clusters to mcb > - FsRtlAddLargeMcbEntry(&DataRunsMCB, > - NextVBN, > - NextAssignedCluster, > - RunLength); > + _SEH2_TRY{ > + if (!FsRtlAddLargeMcbEntry(&DataRunsMCB, > + NextVBN, > + NextAssignedCluster, > + RunLength)) > + { > + FsRtlUninitializeLargeMcb(&DataRunsMCB); > + ExFreePoolWithTag(RunBuffer, TAG_NTFS); > + return STATUS_INSUFFICIENT_RESOURCES; > + } > + } _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) { > + FsRtlUninitializeLargeMcb(&DataRunsMCB); > + ExFreePoolWithTag(RunBuffer, TAG_NTFS); > + _SEH2_YIELD(return STATUS_INSUFFICIENT_RESOURCES); > + } _SEH2_END; Purely cosmetic one here. You should, in your try block, call ExRaiseStatus with the appropriate status and do the cleanup in the exception handler. That avoids code duplication. This also implies you don't want to force the status code, but rather use _SEH2_GetExceptionCode(). Also note that FsRtlAddLargeMcbEntry may fail for various reasons. I don't really know which status code to use as you have no way to know why it failed. -- Pierre Schweitzer <pierre at reactos.org> System & Network Administrator Senior Kernel Developer ReactOS Deutschland e.V.

1 0

Re: [ros-dev] [ros-diffs] [tthompson] 71957: [NTFS] Address some minor issues with attrib.c: Fix gcc build. Fix formatting.
by Pierre Schweitzer 24 Jul '16

24 Jul '16

Comment inline. Le 17/07/2016 à 16:04, tthompson(a)svn.reactos.org a écrit : > Author: tthompson > Date: Sun Jul 17 14:04:01 2016 > New Revision: 71957 > > URL: http://svn.reactos.org/svn/reactos?rev=71957&view=rev > Log: > [NTFS] > Address some minor issues with attrib.c: Fix gcc build. Fix formatting. > > Modified: > branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c > > Modified: branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c > URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/NTFS/drivers/filesyst… > ============================================================================== > --- branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c [iso-8859-1] (original) > +++ branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c [iso-8859-1] Sun Jul 17 14:04:01 2016 > @@ -295,14 +291,14 @@ > ULONG RunBufferOffset = 0; > LONGLONG DataRunOffset; > ULONGLONG LastLCN = 0; > - > LONGLONG Vbn, Lbn, Count; > + int i; Use MS portable type, please (ULONG looks good :-)) -- Pierre Schweitzer <pierre at reactos.org> System & Network Administrator Senior Kernel Developer ReactOS Deutschland e.V.

1 0

Re: [ros-dev] [ros-diffs] [tthompson] 71968: [NTFS] +FreeClusters(). Fix a DPRINT.
by Pierre Schweitzer 24 Jul '16

24 Jul '16

Comment inline. Le 19/07/2016 à 17:31, tthompson(a)svn.reactos.org a écrit : > Author: tthompson > Date: Tue Jul 19 15:31:22 2016 > New Revision: 71968 > > URL: http://svn.reactos.org/svn/reactos?rev=71968&view=rev > Log: > [NTFS] > +FreeClusters(). Fix a DPRINT. > > Modified: > branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c > branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/mft.c > branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/ntfs.h > > Modified: branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c > URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/NTFS/drivers/filesyst… > ============================================================================== > --- branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c [iso-8859-1] (original) > +++ branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/attrib.c [iso-8859-1] Tue Jul 19 15:31:22 2016 > @@ -409,6 +409,208 @@ > return TRUE; > } > > +/** > +* @name FreeClusters > +* @implemented > +* > +* Shrinks the allocation size of a non-resident attribute by a given number of clusters. > +* Frees the clusters from the volume's $BITMAP file as well as the attribute's data runs. > +* > +* @param Vcb > +* Pointer to an NTFS_VCB for the destination volume. > +* > +* @param AttrContext > +* Pointer to an NTFS_ATTR_CONTEXT describing the attribute from which the clusters will be freed. > +* > +* @param AttrOffset > +* Byte offset of the destination attribute relative to its file record. > +* > +* @param FileRecord > +* Pointer to a complete copy of the file record containing the attribute. Must be at least > +* Vcb->NtfsInfo.BytesPerFileRecord bytes long. > +* > +* @param ClustersToFree > +* Number of clusters that should be freed from the end of the data stream. Must be no more > +* Than the number of clusters assigned to the attribute (HighestVCN + 1). > +* > +* @return > +* STATUS_SUCCESS on success. STATUS_INVALID_PARAMETER if AttrContext describes a resident attribute, > +* or if the caller requested more clusters be freed than the attribute has been allocated. > +* STATUS_INSUFFICIENT_RESOURCES if ConvertDataRunsToLargeMCB() fails. > +* STATUS_BUFFER_TOO_SMALL if ConvertLargeMCBToDataRuns() fails. > +* > +* > +*/ > +NTSTATUS > +FreeClusters(PNTFS_VCB Vcb, > + PNTFS_ATTR_CONTEXT AttrContext, > + ULONG AttrOffset, > + PFILE_RECORD_HEADER FileRecord, > + ULONG ClustersToFree) > +{ > + NTSTATUS Status = STATUS_SUCCESS; > + ULONG ClustersLeftToFree = ClustersToFree; > + > + // convert data runs to mcb > + PUCHAR DataRun = (PUCHAR)&AttrContext->Record + AttrContext->Record.NonResident.MappingPairsOffset; > + PNTFS_ATTR_RECORD DestinationAttribute = (PNTFS_ATTR_RECORD)((ULONG_PTR)FileRecord + AttrOffset); > + LARGE_MCB DataRunsMCB; > + ULONG NextAttributeOffset = AttrOffset + AttrContext->Record.Length; > + PNTFS_ATTR_RECORD NextAttribute = (PNTFS_ATTR_RECORD)((ULONG_PTR)FileRecord + NextAttributeOffset); > + ULONGLONG NextVBN = AttrContext->Record.NonResident.LowestVCN; > + > + // Allocate some memory for the RunBuffer > + PUCHAR RunBuffer = ExAllocatePoolWithTag(NonPagedPool, Vcb->NtfsInfo.BytesPerFileRecord, TAG_NTFS); > + ULONG RunBufferOffset = 0; Two notes regarding this allocation: - You should check allocation worked (obviously ;-)); - You should start memory work only after checking the record is not resident. -- Pierre Schweitzer <pierre at reactos.org> System & Network Administrator Senior Kernel Developer ReactOS Deutschland e.V.

1 0

Re: [ros-dev] [ros-diffs] [zhu] 71865: Structural, style, correctness fixes.
by Thomas Faber 09 Jul '16

09 Jul '16

On 2016-07-08 19:52, zhu(a)svn.reactos.org wrote: > --- branches/GSoC_2016/lwIP-tcpip/drivers/network/tcpip/address.c [iso-8859-1] (original) > +++ branches/GSoC_2016/lwIP-tcpip/drivers/network/tcpip/address.c [iso-8859-1] Fri Jul 8 17:52:42 2016 > @@ -33,6 +33,8 @@ > static LIST_ENTRY AddressListHead; > > /* implementation in testing */ > +/* Must already hold the Context->RequestListLock */ > +/* Context should be in ->FileObject->FsContext */ You know there is a way to actually tell that to MSVC's static analyzer, right? ;) You want something like _IRQL_requires_(DISPATCH_LEVEL) _IRQL_requires_same_ _Requires_lock_held_(((PTCP_CONTEXT)IrpSp->FileObject->FsContext)->RequestListLock) The last one might be a little too complicated to actually include, but the IRQL ones are easy to help indicate things like this. > NTSTATUS > PrepareIrpForCancel( > PIRP Irp,

1 0

Re: [ros-dev] [ros-diffs] [gedmurphy] 71862: [NTOS] - Implement IoCreateFileSpecifyDeviceObjectHint - will be needed for fltmgr - Rename IoCreateFile to IopCreateFile and forward both IoCreateFile and IoCreateFileSpecifyDev...
by Alex Ionescu 08 Jul '16

08 Jul '16

On Fri, Jul 8, 2016 at 6:18 AM, <gedmurphy(a)svn.reactos.org> wrote: > > + if (OpenPacket->InternalFlags & IOP_USE_TOP_LEVEL_DEVICE_HINT) > + { > + // FIXME: Verify our device object is good to use > + ASSERT(DirectOpen == FALSE); > } You probably want to take a look at IopCheckTopDeviceHint Best regards, Alex Ionescu

1 0

Re: [ros-dev] [ros-diffs] [hbelusca] 71819: [RTL]: Follow what I guess might be Alex' coding standards.
by Alex Ionescu 08 Jul '16

08 Jul '16

I agree with one-line ifs FWIW... As for the previous code, I think if the file is using separate braces, you shouldn't go around and write new code that follows the no-braces style -- which yes, we do accept. Best regards, Alex Ionescu On Thu, Jul 7, 2016 at 5:25 PM, Timo Kreuzer <timo.kreuzer(a)web.de> wrote: > > According to the "official" ReactOS coding standards, the previous code > was perfectly fine. > Code like "If (Value == 0) Value = FOO;" (in one line) does not match our > coding style rules. (And it's also annoying, when stepping through it with > WinDbg.) > 3 line comments also don't match our coding style :-P > > For reference: https://reactos.org/wiki/Coding_Style > > Timo > > > > Am 05.07.2016 um 16:30 schrieb Alex Ionescu: > > Coding Style - ReactOS Wiki > <https://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=2&ved=0ahUKEw…> > > Stop calling them "mine". > > Best regards, > Alex Ionescu > > On Mon, Jul 4, 2016 at 5:38 PM, <hbelusca(a)svn.reactos.org> wrote: > >> Author: hbelusca >> Date: Tue Jul 5 00:38:28 2016 >> New Revision: 71819 >> >> URL: http://svn.reactos.org/svn/reactos?rev=71819&view=rev >> Log: >> [RTL]: Follow what I guess might be Alex' coding standards. >> >> Modified: >> trunk/reactos/sdk/lib/rtl/process.c >> >> Modified: trunk/reactos/sdk/lib/rtl/process.c >> URL: >> http://svn.reactos.org/svn/reactos/trunk/reactos/sdk/lib/rtl/process.c?rev=… >> >> ============================================================================== >> --- trunk/reactos/sdk/lib/rtl/process.c [iso-8859-1] (original) >> +++ trunk/reactos/sdk/lib/rtl/process.c [iso-8859-1] Tue Jul 5 00:38:28 >> 2016 >> @@ -75,7 +75,7 @@ >> PVOID BaseAddress = NULL; >> SIZE_T EnviroSize; >> SIZE_T Size; >> - PWCHAR Environment = 0; >> + PWCHAR Environment = NULL; >> DPRINT("RtlpInitEnvironment(ProcessHandle: %p, Peb: %p Params: >> %p)\n", >> ProcessHandle, Peb, ProcessParameters); >> >> @@ -301,7 +301,9 @@ >> DUPLICATE_SAME_ACCESS | >> DUPLICATE_SAME_ATTRIBUTES); >> if (!NT_SUCCESS(Status)) >> + { >> _SEH2_LEAVE; >> + } >> } >> >> if (ProcessParameters->StandardOutput) >> @@ -315,7 +317,9 @@ >> DUPLICATE_SAME_ACCESS | >> DUPLICATE_SAME_ATTRIBUTES); >> if (!NT_SUCCESS(Status)) >> + { >> _SEH2_LEAVE; >> + } >> } >> >> if (ProcessParameters->StandardError) >> @@ -329,7 +333,9 @@ >> DUPLICATE_SAME_ACCESS | >> DUPLICATE_SAME_ATTRIBUTES); >> if (!NT_SUCCESS(Status)) >> + { >> _SEH2_LEAVE; >> + } >> } >> } >> _SEH2_FINALLY >> >> >> > > > _______________________________________________ > Ros-dev mailing listRos-dev@reactos.orghttp://www.reactos.org/mailman/listinfo/ros-dev > > > > _______________________________________________ > Ros-dev mailing list > Ros-dev(a)reactos.org > http://www.reactos.org/mailman/listinfo/ros-dev > >

1 0

Re: [ros-dev] [ros-diffs] [hbelusca] 71864: [FLTMGR]: Remove a MSVC warning: FltObjectDereference, whose prototype is publicly defined in fltkernel.h, doesn't return any value.
by Ged Murphy 08 Jul '16

08 Jul '16

This change in incorrect, FltObectDereference does return a value. It's the public definition that's does match the function. -----Original Message----- From: Ros-diffs [mailto:ros-diffs-bounces@reactos.org] On Behalf Of hbelusca(a)svn.reactos.org Sent: 08 July 2016 18:45 To: ros-diffs(a)reactos.org Subject: [ros-diffs] [hbelusca] 71864: [FLTMGR]: Remove a MSVC warning: FltObjectDereference, whose prototype is publicly defined in fltkernel.h, doesn't return any value. Author: hbelusca Date: Fri Jul 8 17:44:59 2016 New Revision: 71864 URL: http://svn.reactos.org/svn/reactos?rev=71864&view=rev Log: [FLTMGR]: Remove a MSVC warning: FltObjectDereference, whose prototype is publicly defined in fltkernel.h, doesn't return any value.

1 0

Re: [ros-dev] [ros-diffs] [hbelusca] 71819: [RTL]: Follow what I guess might be Alex' coding standards.
by Alex Ionescu 08 Jul '16

08 Jul '16

Coding Style - ReactOS Wiki <https://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=2&ved=0ahUKEw…> Stop calling them "mine". Best regards, Alex Ionescu On Mon, Jul 4, 2016 at 5:38 PM, <hbelusca(a)svn.reactos.org> wrote: > Author: hbelusca > Date: Tue Jul 5 00:38:28 2016 > New Revision: 71819 > > URL: http://svn.reactos.org/svn/reactos?rev=71819&view=rev > Log: > [RTL]: Follow what I guess might be Alex' coding standards. > > Modified: > trunk/reactos/sdk/lib/rtl/process.c > > Modified: trunk/reactos/sdk/lib/rtl/process.c > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/sdk/lib/rtl/process.c?rev=… > > ============================================================================== > --- trunk/reactos/sdk/lib/rtl/process.c [iso-8859-1] (original) > +++ trunk/reactos/sdk/lib/rtl/process.c [iso-8859-1] Tue Jul 5 00:38:28 > 2016 > @@ -75,7 +75,7 @@ > PVOID BaseAddress = NULL; > SIZE_T EnviroSize; > SIZE_T Size; > - PWCHAR Environment = 0; > + PWCHAR Environment = NULL; > DPRINT("RtlpInitEnvironment(ProcessHandle: %p, Peb: %p Params: %p)\n", > ProcessHandle, Peb, ProcessParameters); > > @@ -301,7 +301,9 @@ > DUPLICATE_SAME_ACCESS | > DUPLICATE_SAME_ATTRIBUTES); > if (!NT_SUCCESS(Status)) > + { > _SEH2_LEAVE; > + } > } > > if (ProcessParameters->StandardOutput) > @@ -315,7 +317,9 @@ > DUPLICATE_SAME_ACCESS | > DUPLICATE_SAME_ATTRIBUTES); > if (!NT_SUCCESS(Status)) > + { > _SEH2_LEAVE; > + } > } > > if (ProcessParameters->StandardError) > @@ -329,7 +333,9 @@ > DUPLICATE_SAME_ACCESS | > DUPLICATE_SAME_ATTRIBUTES); > if (!NT_SUCCESS(Status)) > + { > _SEH2_LEAVE; > + } > } > } > _SEH2_FINALLY > > >

2 1

Re: [ros-dev] [ros-diffs] [tthompson] 71807: [NTFS][FREELDR] Fix ReadAttribute() and NtfsReadAttribute() in the case when an attribute contains two data runs; Remove extra if statement that prevents second data run from bei...
by Pierre Schweitzer 06 Jul '16

06 Jul '16

Actually, this (duplicated) function looks quite poorly designed (not your fault, obviously :-p). The more you fix it, the more it seems natural that the first specific case should be integrated in the while loop for real. That would make the function more robust, with less duplicated code (and thus, less bugs :-p). GSoC note: this is not a "todo" task. This is just an open discussion about a possible improvement if some day someone's bored. Le 04/07/2016 19:02, tthompson(a)svn.reactos.org a écrit : > Author: tthompson > Date: Mon Jul 4 17:02:10 2016 > New Revision: 71807 > > URL: http://svn.reactos.org/svn/reactos?rev=71807&view=rev > Log: > [NTFS][FREELDR] > Fix ReadAttribute() and NtfsReadAttribute() in the case when an attribute contains two data runs; Remove extra if statement that prevents second data run from being read after it's decoded. > > Modified: > branches/GSoC_2016/NTFS/boot/freeldr/freeldr/lib/fs/ntfs.c > branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/mft.c > > Modified: branches/GSoC_2016/NTFS/boot/freeldr/freeldr/lib/fs/ntfs.c > URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/NTFS/boot/freeldr/fre… > ============================================================================== > --- branches/GSoC_2016/NTFS/boot/freeldr/freeldr/lib/fs/ntfs.c [iso-8859-1] (original) > +++ branches/GSoC_2016/NTFS/boot/freeldr/freeldr/lib/fs/ntfs.c [iso-8859-1] Mon Jul 4 17:02:10 2016 > @@ -320,9 +320,6 @@ > } > else > DataRunStartLCN = -1; > - > - if (*DataRun == 0) > - return AlreadyRead; > } > > while (Length > 0) > > Modified: branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/mft.c > URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/NTFS/drivers/filesyst… > ============================================================================== > --- branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/mft.c [iso-8859-1] (original) > +++ branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/mft.c [iso-8859-1] Mon Jul 4 17:02:10 2016 > @@ -384,9 +384,6 @@ > } > else > DataRunStartLCN = -1; > - > - if (*DataRun == 0) > - return AlreadyRead; > } > > while (Length > 0) > > -- Pierre Schweitzer <pierre at reactos.org> System & Network Administrator Senior Kernel Developer ReactOS Deutschland e.V.

1 0

Re: [ros-dev] [ros-diffs] [tthompson] 71820: [NTFS] Add ability to write to resident attributes. SetAttributeDataLength() - Check if the file is memory mapped before truncating +InternalSetResidentAttributeLength() - Used b...
by Pierre Schweitzer 06 Jul '16

06 Jul '16

Hi, Comments inline. Le 05/07/2016 09:00, tthompson(a)svn.reactos.org a écrit : > Modified: branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/mft.c > URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/NTFS/drivers/filesyst… > ============================================================================== > --- branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/mft.c [iso-8859-1] (original) > +++ branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/mft.c [iso-8859-1] Tue Jul 5 07:00:43 2016 > @@ -170,6 +172,50 @@ > return AttrRecord->Resident.ValueLength; > } > > +void > +InternalSetResidentAttributeLength(PNTFS_ATTR_CONTEXT AttrContext, > + PFILE_RECORD_HEADER FileRecord, > + ULONG AttrOffset, > + ULONG DataSize) > +{ > + ULONG EndMarker = AttributeEnd; > + ULONG FinalMarker = FILE_RECORD_END; > + ULONG NextAttributeOffset; > + ULONG Offset; > + USHORT Padding; > + > + DPRINT("InternalSetResidentAttributeLength( %p, %p, %lu, %lu )\n", AttrContext, FileRecord, AttrOffset, DataSize); > + > + // update ValueLength Field > + AttrContext->Record.Resident.ValueLength = DataSize; > + Offset = AttrOffset + FIELD_OFFSET(NTFS_ATTR_RECORD, Resident.ValueLength); > + RtlCopyMemory((PCHAR)FileRecord + Offset, &DataSize, sizeof(ULONG)); That part looks a bit over-engineered to me and might be simplified (like not using an intermediate var, nor RtlCopyMemory). Cosmetic, I'd replace PCHAR by ULONG_PTR. Same comment goes to code below doing basically the same thing. > + > + // calculate the record length and end marker offset > + AttrContext->Record.Length = DataSize + AttrContext->Record.Resident.ValueOffset; > + NextAttributeOffset = AttrOffset + AttrContext->Record.Length; > + > + // Ensure NextAttributeOffset is aligned to an 8-byte boundary > + if (NextAttributeOffset % 8 != 0) > + { > + Padding = 8 - (NextAttributeOffset % 8); > + NextAttributeOffset += Padding; > + AttrContext->Record.Length += Padding; > + } > + > + // update the record length > + Offset = AttrOffset + FIELD_OFFSET(NTFS_ATTR_RECORD, Length); > + RtlCopyMemory((PCHAR)FileRecord + Offset, &AttrContext->Record.Length, sizeof(ULONG)); > + > + // write the end marker > + RtlCopyMemory((PCHAR)FileRecord + NextAttributeOffset, &EndMarker, sizeof(ULONG)); > + > + // write the final marker > + Offset = NextAttributeOffset + sizeof(ULONG); > + RtlCopyMemory((PCHAR)FileRecord + Offset, &FinalMarker, sizeof(ULONG)); > + > + FileRecord->BytesInUse = Offset + sizeof(ULONG); > +} > > NTSTATUS > SetAttributeDataLength(PFILE_OBJECT FileObject, > @@ -179,6 +225,18 @@ > PFILE_RECORD_HEADER FileRecord, > PLARGE_INTEGER DataSize) > { > + NTSTATUS Status = STATUS_SUCCESS; > + > + // are we truncating the file? > + if (DataSize->QuadPart < AttributeDataLength(&AttrContext->Record) Unless I'm blind, I'm not sure it builds. > + { > + if (!MmCanFileBeTruncated(FileObject->SectionObjectPointer, (PLARGE_INTEGER)&AllocationSize)) > + { > + DPRINT1("Can't truncate a memory-mapped file!\n"); > + return STATUS_USER_MAPPED_FILE; > + } > + } > + > if (AttrContext->Record.IsNonResident) > { > ULONG BytesPerCluster = Fcb->Vcb->NtfsInfo.BytesPerCluster; > Modified: branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/volinfo.c > URL: http://svn.reactos.org/svn/reactos/branches/GSoC_2016/NTFS/drivers/filesyst… > ============================================================================== > --- branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/volinfo.c [iso-8859-1] (original) > +++ branches/GSoC_2016/NTFS/drivers/filesystems/ntfs/volinfo.c [iso-8859-1] Tue Jul 5 07:00:43 2016 > @@ -158,7 +157,7 @@ > DPRINT1("Total clusters in bitmap: %I64x\n", BitmapDataSize * 8); > DPRINT1("Diff in size: %I64d B\n", ((BitmapDataSize * 8) - DeviceExt->NtfsInfo.ClusterCount) * DeviceExt->NtfsInfo.SectorsPerCluster * DeviceExt->NtfsInfo.BytesPerSector); > > - ReadAttribute(DeviceExt, DataContext, Read, (PCHAR)((ULONG_PTR)BitmapData + Read), (ULONG)BitmapDataSize); > + ReadAttribute(DeviceExt, DataContext, 0, (PCHAR)BitmapData, (ULONG)BitmapDataSize); What's this change? > > RtlInitializeBitMap(&Bitmap, (PULONG)BitmapData, DeviceExt->NtfsInfo.ClusterCount); > FreeClusters = RtlNumberOfClearBits(&Bitmap); > > -- Pierre Schweitzer <pierre at reactos.org> System & Network Administrator Senior Kernel Developer ReactOS Deutschland e.V.

2 2

Re: [ros-dev] [ros-diffs] [hbelusca] 71829: [CLIPBRD]: Use NULL for null pointer. [EVENTVWR]: Add folder icons (needed for later).
by Thomas Faber 06 Jul '16

06 Jul '16

Can't event viewer load those icons from shell32 (much like regedit should)? On 2016-07-06 01:42, hbelusca(a)svn.reactos.org wrote: > Author: hbelusca > Date: Tue Jul 5 23:42:40 2016 > New Revision: 71829 > > URL: http://svn.reactos.org/svn/reactos?rev=71829&view=rev > Log: > [CLIPBRD]: Use NULL for null pointer. > [EVENTVWR]: Add folder icons (needed for later). > > Added: > trunk/reactos/base/applications/mscutils/eventvwr/res/folder.ico > - copied unchanged from r71828, trunk/reactos/base/applications/regedit/res/folder.ico > trunk/reactos/base/applications/mscutils/eventvwr/res/folderopen.ico > - copied unchanged from r71828, trunk/reactos/base/applications/regedit/res/folderopen.ico > Modified: > trunk/reactos/base/applications/clipbrd/clipbrd.c > > Modified: trunk/reactos/base/applications/clipbrd/clipbrd.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/applications/clipbrd/… > ============================================================================== > --- trunk/reactos/base/applications/clipbrd/clipbrd.c [iso-8859-1] (original) > +++ trunk/reactos/base/applications/clipbrd/clipbrd.c [iso-8859-1] Tue Jul 5 23:42:40 2016 > @@ -256,12 +256,12 @@ > > case CMD_ABOUT: > { > + HICON hIcon; > WCHAR szTitle[MAX_STRING_LEN]; > - HICON hIcon; > > hIcon = LoadIconW(Globals.hInstance, MAKEINTRESOURCE(CLIPBRD_ICON)); > LoadStringW(Globals.hInstance, STRING_CLIPBOARD, szTitle, ARRAYSIZE(szTitle)); > - ShellAboutW(Globals.hMainWnd, szTitle, 0, hIcon); > + ShellAboutW(Globals.hMainWnd, szTitle, NULL, hIcon); > DeleteObject(hIcon); > break; > } > >

1 0

Re: [ros-dev] [ros-diffs] [zhu] 71823: Fixed crash on client and server exit: Corrected some NULL data marking and checking. Moved some cleanup code so they actually execute on function failure (incomplete). Removed redunda...
by Thomas Faber 05 Jul '16

05 Jul '16

Comments inline. On 2016-07-05 20:35, zhu(a)svn.reactos.org wrote: > --- branches/GSoC_2016/lwIP-tcpip/drivers/network/tcpip/address.c [iso-8859-1] (original) > +++ branches/GSoC_2016/lwIP-tcpip/drivers/network/tcpip/address.c [iso-8859-1] Tue Jul 5 18:35:17 2016 > @@ -357,15 +356,15 @@ > > KeReleaseSpinLockFromDpcLevel(&Context->RequestListLock); > > - if (Context->lwip_tcp_pcb != Context->AddressFile->lwip_tcp_pcb) > + if (Context->lwip_tcp_pcb == Context->AddressFile->lwip_tcp_pcb); You really don't want a semicolon here ;) > { > - tcp_close(Context->lwip_tcp_pcb); > + Context->AddressFile->lwip_tcp_pcb = NULL; > } > + if (!p) > + { > + CopiedLength = 0; > + Status = STATUS_ADDRESS_CLOSED; > + goto BAD; > + } > > if (RemainingDestBytes <= p->len) > { > RtlCopyMemory(CurrentDestLocation, p->payload, RemainingDestBytes); > CopiedLength = RemainingDestBytes; > + Status = STATUS_SUCCESS; > goto RETURN; We normally call labels things like "Exit" and "Failure". > @@ -826,23 +846,24 @@ > > tcp_recved(tpcb, CopiedLength); > I find it confusing not to have Entry = Head->Flink right here. I'm not sure where it is, but unless there's a good reason for it to be somewhere else I'd recommend putting it right in front of the loop. > + while (Entry != Head) > + { > + Request = CONTAINING_RECORD(Entry, TCP_REQUEST, ListEntry); Entry = Entry->Flink? ;) > + if (Request->PendingMode == TCP_REQUEST_PENDING_RECEIVE) > + { > + tcp_recv(tpcb, lwip_tcp_receive_callback); > + break; > + } > + } > @@ -1666,6 +1690,21 @@ > if (!(IsListEmpty(&Context->RequestListHead))) > { > DPRINT1("Disassociating context with outstanding requests\n"); > + Head = &Context->RequestListHead; > + Entry = Head->Flink; > + while (Entry != Head) > + { > + Request = CONTAINING_RECORD(Entry, TCP_REQUEST, ListEntry); Entry = Entry->Flink; > + if (Request->PendingIrp) > + { > + IrpSp = IoGetCurrentIrpStackLocation(Request->PendingIrp); > + DPRINT1("Pending IRP Control Code: %08x\n", IrpSp->MinorFunction); > + } > + else > + { > + DPRINT1("IRP is NULL\n"); > + } > + } > } > KeReleaseSpinLockFromDpcLevel(&Context->RequestListLock); > Thanks. -Thomas

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-dev