Re: [ros-dev] [ros-diffs] [jgardou] 65140: [FASTFAT] - Fix renaming files in case the target file already exists. - Plug a few reference count leaks on FCBs. CORE-8660 #comment fixed in r65140 #resolve CORE-8633 #comment fi...

[Pierre Schweitzer]

Warm thanks for debugging this issue Jérôme. I do value your help.

This should unlock many autoupdating apps in ReactOS.

Thanks again.

On 31/10/2014 12:04, jgardou(a)svn.reactos.org wrote:
>   Author: jgardou
>  Date: Fri Oct 31 11:04:12 2014
>  New Revision: 65140
>  
>  URL: http://svn.reactos.org/svn/reactos?rev=65140&view=rev
>  Log:
>  [FASTFAT]
>   - Fix renaming files in case the target file already exists.
>   - Plug a few reference count leaks on FCBs.
>  CORE-8660 #comment fixed in r65140 #resolve
>  CORE-8633 #comment fixed in r65140 #resolve
>  CORE-4758 #comment fixed in r65140 #resolve
>  
>  Modified:
>      trunk/reactos/drivers/filesystems/fastfat/cleanup.c
>      trunk/reactos/drivers/filesystems/fastfat/create.c
>      trunk/reactos/drivers/filesystems/fastfat/finfo.c
>  
>  Modified: trunk/reactos/drivers/filesystems/fastfat/cleanup.c
>  URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/fastfa…
>  ==============================================================================
>  --- trunk/reactos/drivers/filesystems/fastfat/cleanup.c	[iso-8859-1] (original)
>  +++ trunk/reactos/drivers/filesystems/fastfat/cleanup.c	[iso-8859-1] Fri Oct 31 11:04:12
> 2014
>  @@ -92,6 +92,7 @@
>                   pFcb->FileObject = NULL;
>                   CcUninitializeCacheMap(tmpFileObject, NULL, NULL);
>                   ObDereferenceObject(tmpFileObject);
>  +                vfatReleaseFCB(IrpContext->DeviceExt, pFcb);
>               }
>   
>               CcPurgeCacheSection(FileObject->SectionObjectPointer, NULL, 0, FALSE);
>  
>  Modified: trunk/reactos/drivers/filesystems/fastfat/create.c
>  URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/fastfa…
>  ==============================================================================
>  --- trunk/reactos/drivers/filesystems/fastfat/create.c	[iso-8859-1] (original)
>  +++ trunk/reactos/drivers/filesystems/fastfat/create.c	[iso-8859-1] Fri Oct 31 11:04:12
> 2014
>  @@ -365,7 +365,6 @@
>           DPRINT("'%wZ'\n",
> &FileObject->RelatedFileObject->FileName);
>   
>           *ParentFcb = FileObject->RelatedFileObject->FsContext;
>  -        vfatGrabFCB(DeviceExt, *ParentFcb);
>       }
>       else
>       {
>  
>  Modified: trunk/reactos/drivers/filesystems/fastfat/finfo.c
>  URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/filesystems/fastfa…
>  ==============================================================================
>  --- trunk/reactos/drivers/filesystems/fastfat/finfo.c	[iso-8859-1] (original)
>  +++ trunk/reactos/drivers/filesystems/fastfat/finfo.c	[iso-8859-1] Fri Oct 31 11:04:12
> 2014
>  @@ -396,41 +396,53 @@
>       /* If it exists */
>       if (NT_SUCCESS(Status))
>       {
>  +        DPRINT("Target file %wZ exists. FCB Flags %08x\n", NewName,
> TargetFcb->Flags);
>           /* Check whether we are allowed to replace */
>           if (ReplaceIfExists)
>           {
>               /* If that's a directory or a read-only file, we're not allowed */
>  -            if (vfatFCBIsDirectory(TargetFcb) || ((*TargetFcb->Attributes &
> FILE_ATTRIBUTE_READONLY) == FILE_ATTRIBUTE_READONLY));
>  -            {
>  +            if (vfatFCBIsDirectory(TargetFcb) || ((*TargetFcb->Attributes &
> FILE_ATTRIBUTE_READONLY) == FILE_ATTRIBUTE_READONLY))
>  +            {
>  +                DPRINT("And this is a readonly file!\n");
>                   vfatReleaseFCB(DeviceExt, *ParentFCB);
>                   *ParentFCB = NULL;
>                   vfatReleaseFCB(DeviceExt, TargetFcb);
>                   return STATUS_OBJECT_NAME_COLLISION;
>               }
>   
>  -            /* Attempt to flush (might close the file) */
>  -            if (!MmFlushImageSection(TargetFcb->FileObject->SectionObjectPointer,
> MmFlushForDelete))
>  -            {
>  +
>  +            /* If we still have a file object, close it. */
>  +            if (TargetFcb->FileObject)
>  +            {
>  +                if
> (!MmFlushImageSection(TargetFcb->FileObject->SectionObjectPointer,
> MmFlushForDelete))
>  +                {
>  +                    DPRINT("MmFlushImageSection failed.\n");
>  +                    vfatReleaseFCB(DeviceExt, *ParentFCB);
>  +                    *ParentFCB = NULL;
>  +                    vfatReleaseFCB(DeviceExt, TargetFcb);
>  +                    return STATUS_ACCESS_DENIED;
>  +                }
>  +
>  +                TargetFcb->FileObject->DeletePending = TRUE;
>  +                VfatCloseFile(DeviceExt, TargetFcb->FileObject);
>  +            }
>  +
>  +            /* If we are here, ensure the file isn't open by anyone! */
>  +            if (TargetFcb->OpenHandleCount != 0)
>  +            {
>  +                DPRINT("There are still open handles for this file.\n");
>                   vfatReleaseFCB(DeviceExt, *ParentFCB);
>                   *ParentFCB = NULL;
>                   vfatReleaseFCB(DeviceExt, TargetFcb);
>                   return STATUS_ACCESS_DENIED;
>               }
>   
>  -            /* If we are, ensure the file isn't open by anyone! */
>  -            if (TargetFcb->OpenHandleCount != 0)
>  -            {
>  -                vfatReleaseFCB(DeviceExt, *ParentFCB);
>  -                *ParentFCB = NULL;
>  -                vfatReleaseFCB(DeviceExt, TargetFcb);
>  -                return STATUS_ACCESS_DENIED;
>  -            }
>  -
>               /* Effectively delete old file to allow renaming */
>  +            DPRINT("Effectively deleting the file.\n");
>               VfatDelEntry(DeviceExt, TargetFcb, NULL);
>  -            vfatGrabFCB(DeviceExt, *ParentFCB);
>               vfatReleaseFCB(DeviceExt, TargetFcb);
>               *Deleted = TRUE;
>  +            return STATUS_SUCCESS;
>           }
>           else
>           {
>  
>   

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