Ros-dev November 2014

ros-dev@reactos.org

24 participants
27 discussions

Re: [ros-dev] [ros-diffs] [hbelusca] 65337: [NTOS:PNPMGR] - Remove an unneeded ExFreePool(DeviceInstance.Buffer); call in IopGetInterfaceDeviceList because at this point DeviceInstance is not yet initialized. Fixes MSVC bui...
by Timo Kreuzer 09 Nov '14

09 Nov '14

You add parentheses to conform to coding style (our coding style guidelines do not require it) and on the other hand you convert a 2 line if statement into one line, which does not conform to our coding style? Am 09.11.2014 03:26, schrieb hbelusca(a)svn.reactos.org: > Author: hbelusca > Date: Sun Nov 9 02:26:49 2014 > New Revision: 65337 > > URL: http://svn.reactos.org/svn/reactos?rev=65337&view=rev > Log: > [NTOS:PNPMGR] > - Remove an unneeded ExFreePool(DeviceInstance.Buffer); call in IopGetInterfaceDeviceList because at this point DeviceInstance is not yet initialized. Fixes MSVC build. > - No need to check for DeviceInstance.Buffer being NULL or not (in IopDeviceStatus), because in case it was NULL the IopCaptureUnicodeString call already failed. > - Add some brackets to conform to code style. > > Modified: > trunk/reactos/ntoskrnl/io/pnpmgr/plugplay.c > > Modified: trunk/reactos/ntoskrnl/io/pnpmgr/plugplay.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/pnpmgr/plugpla… > ============================================================================== > --- trunk/reactos/ntoskrnl/io/pnpmgr/plugplay.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/io/pnpmgr/plugplay.c [iso-8859-1] Sun Nov 9 02:26:49 2014 > @@ -199,8 +199,7 @@ > } > _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) > { > - if (Name.Buffer) > - ExFreePool(Name.Buffer); > + if (Name.Buffer) ExFreePool(Name.Buffer); > Status = _SEH2_GetExceptionCode(); > } > _SEH2_END; >

1 0

Re: [ros-dev] [ros-diffs] [hbelusca] 65336: [NTVDM]: Use variable-length buffers in DisplayMessage, in case we display a very long message (uses the _vscwprintf CRT function, not available on Win2k. You need to recompile NT...
by Timo Kreuzer 09 Nov '14

09 Nov '14

Am 09.11.2014 02:46, schrieb hbelusca(a)svn.reactos.org: > Author: hbelusca > Date: Sun Nov 9 01:46:31 2014 > New Revision: 65336 > > URL: http://svn.reactos.org/svn/reactos?rev=65336&view=rev > Log: > [NTVDM]: Use variable-length buffers in DisplayMessage, in case we display a very long message (uses the _vscwprintf CRT function, not available on Win2k. You need to recompile NTVDM with WIN2K_COMPLIANT define if you want to be able to run it on win2k. In that case DisplayMessage uses a quite large enough buffer for its needs). If somebody knows an alternative to _vscwprintf that does the very same job, and which exists on Win2k, I would be happy to use it instead. Since _vscwprintf will do the whole work already anyway, you could as well directly print it into the static buffer. You could use _vsnwprintf or StringCbPrintfW, which internally uses the former, and use the result to decide whether the buffer was large enough. This will be more performant, since you only need to print once, when the buffer is large enough.

1 0

Re: [ros-dev] [ros-diffs] [tfaber] 65269: [FASTFAT] - VfatCleanupFile: do not dereference the FCB when closing the volume file object. ObDereferenceObject already causes an IRP_MJ_CLOSE request to be sent, which dereference...
by Thomas Faber 05 Nov '14

05 Nov '14

Oh... Jérôme, I only just saw that you added this in r65140... And my change does break the msi tests again. :\ Any idea what's going wrong here? I'm relatively certain my change is correct because vfatFCBInitializeCacheFromVolume takes one reference, and VfatCloseFile invoked via ObDereferenceObject will remove that reference again. So apparently we're leaking a directory reference somewhere else? :( Not sure if your previous investigation into this issue can provide any pointers. Otherwise I'll have to start digging for the leak all over again... Thanks. -Thomas On 2014-11-05 19:52, tfaber(a)svn.reactos.org wrote: > --- trunk/reactos/drivers/filesystems/fastfat/cleanup.c [iso-8859-1] (original) > +++ trunk/reactos/drivers/filesystems/fastfat/cleanup.c [iso-8859-1] Wed Nov 5 18:52:11 2014 > @@ -92,7 +92,6 @@ > pFcb->FileObject = NULL; > CcUninitializeCacheMap(tmpFileObject, NULL, NULL); > ObDereferenceObject(tmpFileObject); > - vfatReleaseFCB(IrpContext->DeviceExt, pFcb); > } > > CcPurgeCacheSection(FileObject->SectionObjectPointer, NULL, 0, FALSE); > >

2 1

Re: [ros-dev] [ros-diffs] [tkreuzer] 65255: Merged revision(s) 62353, 62531-62532, 62853, 64152, 64173-64174 from branches/kernel-fun/reactos: [NTOSKRNL] Implement SystemPrioritySeperation case in NtSetSystemInformation ......
by Thomas Faber 04 Nov '14

04 Nov '14

On 2014-11-04 21:47, tkreuzer(a)svn.reactos.org wrote: > --- trunk/reactos/ntoskrnl/io/iomgr/file.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/io/iomgr/file.c [iso-8859-1] Tue Nov 4 20:47:18 2014 > @@ -1580,6 +1580,8 @@ > if (OpenPacket.ParseCheck != TRUE) > { > /* Parse failed */ > + DPRINT1("IopQueryAttributesFile failed for '%wZ' with 0x%lx\n", > + ObjectAttributes->ObjectName, Status); > return Status; > } > else This one seems to be super noisy on testbot.

1 0

Re: [ros-dev] [ros-diffs] [jgardou] 65210: [NTOS/PS] - Do not leak a reference to the process object when setting quotas.
by Timo Kreuzer 03 Nov '14

03 Nov '14

Doesn't this change Windows behaviour? No, luckily it does not change *Windows* behaviour. It does not even change ReactOS behaviour, but only, because it was already broken before. Since MS implemented this function in an arbitrary and hard to comprehend way, we need to follow this pattern. This means, the function must not fail on process reference failure, unless certain conditions are met, otherwise we will fail with the wrong Status code. To fix the situation, we could remember the fact that the process couldn't be referenced (possibly by setting Process to NULL) and later check that after other checks were done. The alternative is referencing the process individually in each and every case, which will make the function even more huge and does not really help with it's readability. Or we can factor our each case and implement it as a separate function. It would add a lot of code duplication, but might help a bit with readability and maintainability. Am 03.11.2014 10:52, schrieb jgardou(a)svn.reactos.org: > Author: jgardou > Date: Mon Nov 3 09:52:08 2014 > New Revision: 65210 > > URL: http://svn.reactos.org/svn/reactos?rev=65210&view=rev > Log: > [NTOS/PS] > - Do not leak a reference to the process object when setting quotas. > > Modified: > trunk/reactos/ntoskrnl/include/internal/ps.h > trunk/reactos/ntoskrnl/ps/query.c > trunk/reactos/ntoskrnl/ps/quota.c > > Modified: trunk/reactos/ntoskrnl/include/internal/ps.h > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/… > ============================================================================== > --- trunk/reactos/ntoskrnl/include/internal/ps.h [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/include/internal/ps.h [iso-8859-1] Mon Nov 3 09:52:08 2014 > @@ -303,7 +303,7 @@ > NTSTATUS > NTAPI > PspSetQuotaLimits( > - _In_ HANDLE ProcessHandle, > + _In_ PEPROCESS Process, > _In_ ULONG Unused, > _In_ PVOID QuotaLimits, > _In_ ULONG QuotaLimitsLength, > > Modified: trunk/reactos/ntoskrnl/ps/query.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ps/query.c?rev=65… > ============================================================================== > --- trunk/reactos/ntoskrnl/ps/query.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/ps/query.c [iso-8859-1] Mon Nov 3 09:52:08 2014 > @@ -1528,6 +1528,7 @@ > /* Validate the number */ > if ((BasePriority > HIGH_PRIORITY) || (BasePriority <= LOW_PRIORITY)) > { > + ObDereferenceObject(Process); > return STATUS_INVALID_PARAMETER; > } > > @@ -1918,11 +1919,12 @@ > > case ProcessQuotaLimits: > > - return PspSetQuotaLimits(ProcessHandle, > + Status = PspSetQuotaLimits(Process, > 1, > ProcessInformation, > ProcessInformationLength, > PreviousMode); > + break; > > case ProcessWorkingSetWatch: > DPRINT1("WS watch not implemented\n"); > > Modified: trunk/reactos/ntoskrnl/ps/quota.c > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ps/quota.c?rev=65… > ============================================================================== > --- trunk/reactos/ntoskrnl/ps/quota.c [iso-8859-1] (original) > +++ trunk/reactos/ntoskrnl/ps/quota.c [iso-8859-1] Mon Nov 3 09:52:08 2014 > @@ -292,14 +292,13 @@ > NTSTATUS > NTAPI > PspSetQuotaLimits( > - _In_ HANDLE ProcessHandle, > + _In_ PEPROCESS Process, > _In_ ULONG Unused, > _In_ PVOID QuotaLimits, > _In_ ULONG QuotaLimitsLength, > _In_ KPROCESSOR_MODE PreviousMode) > { > QUOTA_LIMITS_EX CapturedQuotaLimits; > - PEPROCESS Process; > PEPROCESS_QUOTA_BLOCK QuotaBlock, OldQuotaBlock; > BOOLEAN IncreaseOkay; > KAPC_STATE SavedApcState; > @@ -368,19 +367,6 @@ > } > _SEH2_END; > > - /* Reference the process */ > - Status = ObReferenceObjectByHandle(ProcessHandle, > - PROCESS_SET_QUOTA, > - PsProcessType, > - PreviousMode, > - (PVOID*)&Process, > - NULL); > - if (!NT_SUCCESS(Status)) > - { > - DPRINT1("Failed to reference process handle: 0x%lx\n", Status); > - return Status; > - } > - > /* Check the caller changes the working set size limits */ > if ((CapturedQuotaLimits.MinimumWorkingSetSize != 0) && > (CapturedQuotaLimits.MaximumWorkingSetSize != 0)) > @@ -418,7 +404,6 @@ > /* Check if the caller has the required privilege */ > if (!SeSinglePrivilegeCheck(SeIncreaseQuotaPrivilege, PreviousMode)) > { > - ObDereferenceObject(Process); > return STATUS_PRIVILEGE_NOT_HELD; > } > > @@ -460,8 +445,6 @@ > Status = STATUS_SUCCESS; > } > > - /* Dereference the process and return the status */ > - ObDereferenceObject(Process); > return Status; > } > > > >

1 0

Fezile server downtime this evening
by Colin Finck 03 Nov '14

03 Nov '14

Hi all, Jan and I will finally install the remote management card in Fezile this evening and verify the cabling of our UPS systems. This will cause a short downtime of the Fezile server along with its testslaves, cppcheck, Doxygen, Git and iso.reactos.org. In case we notice a fault in UPS cabling, we may also be required to temporarily shut down other servers, replug their power cables and boot them up again. This may cause very short downtimes as well. After this measure, we will be able to totally manage this important server remotely, even if its OS has failed at some point. Proper UPS cabling also ensures smooth operations in case of sudden power outages. Thanks for your understanding! Cheers, Colin

1 0

Re: [ros-dev] [ros-diffs] [pschweitzer] 65166: [NTFS] Simplify buffer size management in GetNtfsFileRecord(). As tests shown, Windows doesn't care about the real size of the file record and expects the caller to pass the co...
by Thomas Faber 01 Nov '14

01 Nov '14

On 2014-11-01 11:02, pschweitzer(a)svn.reactos.org wrote: > - OutputBuffer->FileRecordLength = FileRecord->BytesInUse; > - RtlCopyMemory(OutputBuffer->FileRecordBuffer, FileRecord, FileRecord->BytesInUse); > + OutputBuffer->FileRecordLength = DeviceExt->NtfsInfo.BytesPerFileRecord; > + RtlCopyMemory(OutputBuffer->FileRecordBuffer, FileRecord, DeviceExt->NtfsInfo.BytesPerFileRecord); Wait, now there's no check against OutputBufferLength at all? It should at least be min(DeviceExt->NtfsInfo.BytesPerFileRecord, Stack->Parameters.FileSystemControl.OutputBufferLength) in the memcpy size. Or am I missing something?

2 3

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-dev November 2014