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

[Pierre Schweitzer]

Le 06/07/2016 19:59, Trevor Thompson a écrit :
> Hi Pierre, thanks for the fast review! I'll respond inline.
> 
> On Tue, Jul 5, 2016 at 4:39 PM, Pierre Schweitzer <pierre at reactos.org>
> wrote:
> 
>> Hi,
>>
>> Comments inline.
>>
>> Le 05/07/2016 09:00, tthompson at svn.reactos.org a écrit :
>> ==============================================================================
>>> --- 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?
>>
> 
> I'm getting rid of the Read variable, which isn't being used in any
> meaningful way. This just removes all references to said variable. It was
> just vestigial from this bit of code I copy-pasted from
> NtfsGetFreeClusters():
> 
> /* FIXME: Totally underoptimized! */
>     for (; Read < BitmapDataSize; Read +=
> DeviceExt->NtfsInfo.BytesPerSector)
>     {
>         ReadAttribute(DeviceExt, DataContext, Read,
> (PCHAR)((ULONG_PTR)BitmapData + Read), DeviceExt->NtfsInfo.BytesPerSector);
>     }
> 
> By the way, is there any reason I shouldn't heed the comment and replace
> this with a single call to ReadAttribute()?

I don't recall why I had to do it that way... Perhaps I was hitting a
bug somewhere else (perhaps read alignment issues?). But if it doesn't
regress anything, just feel free to do it properly.

-- 
Pierre Schweitzer <pierre at reactos.org>
System & Network Administrator
Senior Kernel Developer
ReactOS Deutschland e.V.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3960 bytes
Desc: Signature cryptographique S/MIME
URL: <http://www.reactos.org/pipermail/ros-dev/attachments/20160706/5ed64ed2/attachment.bin>