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