Ros-dev July 2016

ros-dev@reactos.org

8 participants
15 discussions

Status Meeting (June 2016)

by Aleksey Bragin

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

8 years, 1 month

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

by Pierre Schweitzer

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.

8 years, 1 month

Re: [ros-dev] [ros-diffs] [tthompson] 71957: [NTFS] Address some minor issues with attrib.c: Fix gcc build. Fix formatting.

by Pierre Schweitzer

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.

8 years, 1 month

Re: [ros-dev] [ros-diffs] [tthompson] 71968: [NTFS] +FreeClusters(). Fix a DPRINT.

by Pierre Schweitzer

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.

8 years, 1 month

Re: [ros-dev] [ros-diffs] [zhu] 71865: Structural, style, correctness fixes.

by Thomas Faber

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,

8 years, 2 months

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

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

8 years, 2 months

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

by Alex Ionescu

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

8 years, 2 months

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

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.

8 years, 2 months

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

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

8 years, 2 months

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

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.

8 years, 2 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-dev July 2016